Re: [tip: sched/core] sched: Fix balance_callback()
tip-bot2 for Peter Zijlstra schreef op wo 11-11-2020 om 08:23 [+]: > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > [...] > +static void do_balance_callbacks(struct rq *rq, struct callback_head *head) > +{ > + void (*func)(struct rq *rq); > + struct callback_head *next; > + > + lockdep_assert_held(&rq->lock); > + > + while (head) { > + func = (void (*)(struct rq *))head->func; > + next = head->next; > + head->next = NULL; > + head = next; Naive question: is there some subtle C-issue that is evaded here by setting head->next to NULL prior to copying over it? (I know this piece of code only got copied around in this patch and this is therefor not something that this patch actually introduced.) > + > + func(rq); > + } > +} Thanks, Paul Bolle
Re: [PATCH 5.9 080/391] ASoC: SOF: fix a runtime pm issue in SOF when HDMI codec doesnt work
Greg Kroah-Hartman schreef op di 03-11-2020 om 21:32 [+0100]: > From: Rander Wang > > [ Upstream commit 6c63c954e1c52f1262f986f36d95f557c6f8fa94 ] > > When hda_codec_probe() doesn't initialize audio component, we disable > the codec and keep going. However,the resources are not released. The > child_count of SOF device is increased in snd_hdac_ext_bus_device_init > but is not decrease in error case, so SOF can't get suspended. > > snd_hdac_ext_bus_device_exit will be invoked in HDA framework if it > gets a error. Now copy this behavior to release resources and decrease > SOF device child_count to release SOF device. > > Signed-off-by: Rander Wang > Reviewed-by: Pierre-Louis Bossart > Reviewed-by: Bard Liao > Reviewed-by: Guennadi Liakhovetski > Signed-off-by: Ranjani Sridharan > Link: > https://lore.kernel.org/r/20200825235040.1586478-3-ranjani.sridha...@linux.intel.com > Signed-off-by: Mark Brown > Signed-off-by: Sasha Levin > --- > sound/soc/sof/intel/hda-codec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c > index 2c5c451fa19d7..c475955c6eeba 100644 > --- a/sound/soc/sof/intel/hda-codec.c > +++ b/sound/soc/sof/intel/hda-codec.c > @@ -151,7 +151,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int > address, > if (!hdev->bus->audio_component) { > dev_dbg(sdev->dev, > "iDisp hw present but no driver\n"); > - return -ENOENT; > + goto error; > } > hda_priv->need_display_power = true; > } > @@ -174,7 +174,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int > address, >* other return codes without modification >*/ > if (ret == 0) > - ret = -ENOENT; > + goto error; > } > > return ret; My local build of v5.9.5 broke on this patch. sound/soc/sof/intel/hda-codec.c: In function 'hda_codec_probe': sound/soc/sof/intel/hda-codec.c:177:4: error: label 'error' used but not defined 177 |goto error; |^~~~ make[4]: *** [scripts/Makefile.build:283: sound/soc/sof/intel/hda-codec.o] Error 1 make[3]: *** [scripts/Makefile.build:500: sound/soc/sof/intel] Error 2 make[2]: *** [scripts/Makefile.build:500: sound/soc/sof] Error 2 make[1]: *** [scripts/Makefile.build:500: sound/soc] Error 2 make: *** [Makefile:1778: sound] Error 2 There's indeed no error label in v5.9.5. (There is one in v5.10-rc2, I just checked.) Is no-one else running into this? Thanks, Paul Bolle
[tip: locking/core] locking/atomics: Check atomic-arch-fallback.h too
The following commit has been merged into the locking/core branch of tip: Commit-ID: d89d5f855f84ccf3f7e648813b4bb95c780bd7cd Gitweb: https://git.kernel.org/tip/d89d5f855f84ccf3f7e648813b4bb95c780bd7cd Author:Paul Bolle AuthorDate:Thu, 01 Oct 2020 22:20:28 +02:00 Committer: Peter Zijlstra CommitterDate: Wed, 07 Oct 2020 18:14:14 +02:00 locking/atomics: Check atomic-arch-fallback.h too The sha1sum of include/linux/atomic-arch-fallback.h isn't checked by check-atomics.sh. It's not clear why it's skipped so let's check it too. Signed-off-by: Paul Bolle Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Mark Rutland Link: https://lkml.kernel.org/r/20201001202028.1048418-1-pebo...@tiscali.nl --- scripts/atomic/check-atomics.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh index 8378c63..82748d4 100755 --- a/scripts/atomic/check-atomics.sh +++ b/scripts/atomic/check-atomics.sh @@ -16,6 +16,7 @@ fi cat <
Re: Build regressions/improvements in v5.9-rc8
Richard Weinberger schreef op wo 07-10-2020 om 14:08 [+0200]: > UML has no ia32 emulation and therefore no in_ia32_syscall(). > Maybe you can check for CONFIG_IA32_EMULATION too? The pending fix is: #if defined(CONFIG_X86_64) && !defined(CONFIG_UML) Since this check guards in_ia32_syscall() just checking CONFIG_IA32_EMULATION should do too. (Way outside my limited expertise, but anyway: is does look odd to see a call to in_ia32_syscall() in drivers/. All other calls are in arch/x86/. Isn't this a bit too x86 specific for an arch independent driver?) Thanks, Paul Bolle
Re: Build regressions/improvements in v5.9-rc8
Andy Shevchenko schreef op wo 07-10-2020 om 14:58 [+0300]: > Does [1] fix the issue? > > [1]: > https://lore.kernel.org/linux-gpio/20201005131044.87276-1-andriy.shevche...@linux.intel.com/ Yes, it fixes the build error. Thanks, Paul Bolle
Re: Build regressions/improvements in v5.9-rc8
[Added Richard and Anton.] Andy Shevchenko schreef op ma 05-10-2020 om 15:58 [+0300]: > On Mon, Oct 05, 2020 at 11:35:33AM +0200, Geert Uytterhoeven wrote: > > On Mon, Oct 5, 2020 at 11:33 AM Geert Uytterhoeven > > wrote: > > > JFYI, when comparing v5.9-rc8[1] to v5.9-rc7[3], the summaries are: > > > - build errors: +3/-6 > > Thanks for the report! > > > + /kisskb/src/drivers/gpio/gpiolib-cdev.c: error: implicit > > declaration of function 'in_ia32_syscall' > > [-Werror=implicit-function-declaration]: => 430:6 > > + /kisskb/src/drivers/gpio/gpiolib-cdev.c: error: unknown type name > > 'compat_u64': => 432:4 > > > > x86_64/um-all{mod,yes}config > > I guess the quick fix is to disable that code for UML, I don't know how IOCTLs > are working in UML in cases when host - guest - guest app either from: > 1. x86_64 - x86_64 - i386; > 2. x86_64 - i386 - i386. I ran into this build error too. Perhaps the UML maintainers have an idea what to do here. The commit that triggers this error is 5ad284ab3a01 ("gpiolib: Fix line event handling in syscall compatible mode"). Thanks, Paul Bolle
[PATCH] locking/atomics: Check atomic-arch-fallback.h too
The sha1sum of include/linux/atomic-arch-fallback.h isn't checked by check-atomics.sh. It's not clear why it's skipped so let's check it too. Signed-off-by: Paul Bolle --- It seems it never has been checked. So this does cast some doubt about the usefulness of these tests. But I'm clueless about this atomic stuff so what do I know? scripts/atomic/check-atomics.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh index 8378c63a1e09..82748d42ecc5 100755 --- a/scripts/atomic/check-atomics.sh +++ b/scripts/atomic/check-atomics.sh @@ -16,6 +16,7 @@ fi cat <
Re: IWL AC 8260, kernel 5.3.*, many kernel WARNING
Norbert Preining schreef op vr 27-09-2019 om 10:04 [+0900]: > Sep 27 09:08:35 burischnitzel kernel: WARNING: CPU: 0 PID: 525 at > iwl_mvm_rx_umac_scan_complete_notif.cold+0xc/0x13 [iwlmvm] > [...] > > This repeats a few times (2-4) and then it settles down. > > WIFI works without any problems, though. This made me check my logs. It turns out that in my current v5.3.1 session (now in its fifth day) there are 128 of these warnings. In my case the warning also mentions drivers/net/wireless/intel/iwlwifi/mvm/scan.c:1874 My previous session (v5.3.0) accumulated 162 of these warnings in about six days, while the session before that (v5.2.y) showed none. Just like in Norbert's case, wifi appears to work fine. Paul Bolle
Re: Linux 5.2.10
Greg KH schreef op ma 26-08-2019 om 06:34 [+0200]: > It's on that key already, have you refreshed your version of it? I have now. sas...@kernel.org (and another identity) is now on that key. Thanks, Paul Bolle
Re: Linux 5.2.10
Sasha, Sasha Levin schreef op zo 25-08-2019 om 10:47 [-0400]: > I'm announcing the release of the 5.2.10 kernel. > > All users of the 5.2 kernel series must upgrade. > > The updated 5.2.y git tree can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > linux-5.2.y > and can be browsed at the normal kernel.org git web browser: > > https://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary v5.2.10 was tagged by sas...@kernel.org but signed by alexander.le...@verizon.com. Perhaps you could use one of gpg2's many options to add an aka "Sasha Levin " line to that key. (I assume "--recv-key" then would have found your key.) Or would that be a sin against kernel.org policy, state-of-the-art cryptographic practices, or whatever? Thanks, Paul Bolle
Re: [PATCH] isdn/gigaset: check endpoint null in gigaset_probe
Hi Phong, Phong Tran schreef op za 27-07-2019 om 08:56 [+0700]: > On 7/26/19 9:22 PM, Paul Bolle wrote: > > Phong Tran schreef op vr 26-07-2019 om 20:35 [+0700]: > > > diff --git a/drivers/isdn/gigaset/usb-gigaset.c > > > b/drivers/isdn/gigaset/usb-gigaset.c > > > index 1b9b43659bdf..2e011f3db59e 100644 > > > --- a/drivers/isdn/gigaset/usb-gigaset.c > > > +++ b/drivers/isdn/gigaset/usb-gigaset.c > > > @@ -703,6 +703,10 @@ static int gigaset_probe(struct usb_interface > > > *interface, > > > usb_set_intfdata(interface, cs); > > > > > > endpoint = &hostif->endpoint[0].desc; > > > +if (!endpoint) { > > > + dev_err(cs->dev, "Couldn't get control endpoint\n"); > > > + return -ENODEV; > > > + } > > > > When can this happen? Is this one of those bugs that one can only trigger > > with > > a specially crafted (evil) usb device? > > > > Yes, in my understanding, this only happens with random test of syzbot. Looking at this again, I note the code is taking the address of a struct usb_endpoint_descriptor that's stored somewhere in memory. That address can't be NULL, can it? So I haven't even looked at the fuzzer's report here, but I don't see how this patch could help. It only adds dead code. Am I missing something and should I drink even more coffee this Saturday morning? > > > buffer_size = le16_to_cpu(endpoint->wMaxPacketSize); > > > ucs->bulk_out_size = buffer_size; > > > @@ -722,6 +726,11 @@ static int gigaset_probe(struct usb_interface > > > *interface, > > > > > Please note that I'm very close to getting cut off from the ISDN network, so > > the chances of being able to testi this on a live system are getting small. > > > > This bug can be invalid now. Do you agree? It's just that your patch arrived while I was busy doing my last ever test of the gigaset driver. So please don't expect me to put much time in this report (see https://lwn.net/ml/linux-kernel/20190726220541.28783-1-pebolle%40tiscali.nl/ ). Thanks, Paul Bolle
[PATCH] gigaset: stop maintaining seperately
The Dutch consumer grade ISDN network will be shut down on September 1, 2019. This means I'll be converted to some sort of VOIP shortly. At that point it would be unwise to try to maintain the gigaset driver, even for odd fixes as I do. So I'll stop maintaining it as a seperate driver and bump support to CAPI in staging. De facto this means the driver will be unmaintained, since no-one seems to be working on CAPI. I've lighty tested the hardware specific modules of this driver (bas-gigaset, ser-gigaset, and usb-gigaset) for v5.3-rc1. The basic functionality appears to be working. It's unclear whether anyone still cares. I'm aware of only one person sort of using the driver a few years ago. Thanks to Karsten Keil for the ISDN subsystems gigaset was using (I4L and CAPI). And many thanks to Hansjoerg Lipp and Tilman Schmidt for writing and upstreaming this driver. Signed-off-by: Paul Bolle --- MAINTAINERS | 7 --- 1 file changed, 7 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 783569e3c4b4..e99afbd13355 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6822,13 +6822,6 @@ F: Documentation/filesystems/gfs2*.txt F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h -GIGASET ISDN DRIVERS -M: Paul Bolle -L: gigaset307x-com...@lists.sourceforge.net -W: http://gigaset307x.sourceforge.net/ -S: Odd Fixes -F: drivers/staging/isdn/gigaset/ - GNSS SUBSYSTEM M: Johan Hovold T: git git://git.kernel.org/pub/scm/linux/kernel/git/johan/gnss.git -- 2.21.0
Re: [VDSO] [x86_32] v5-3-rc1 needs vdso32=0 to get systemd-journald running
Sean Christopherson schreef op vr 26-07-2019 om 09:20 [-0700]: > More than likely it's this: > > https://lkml.kernel.org/r/20190719170343.ga13...@linux.intel.com Yes. systemctl --version prints +SECCOMP so I guess systemd-journald has it enabled too. So I now know which thread I should check. (The subject of that thread contains "[5.2 REGRESSION]", but I'd say "[5.3-rc1 REGRESSION]" would more accurate, but whatever.) Thanks, Paul Bolle
[VDSO] [x86_32] v5-3-rc1 needs vdso32=0 to get systemd-journald running
My first attempts to boot v5.3-rc1 on my (ancient) ThinkPad X41 made systemd- journald crash. I kept ending up with nasty my messages on the console: Starting Journal Service... [...] [7.143552] systemd-journald[213]: Assertion 'clock_gettime(map_clock_id(clock_id), &ts) == 0' failed at ../src/basic/time-util.c:55, function now(). Aborting. [FAILED] Failed to start Journal Service. See 'systemctl status systemd-journald.service' for details. [7.220367] systemd-coredump[217]: Cannot resolve systemd-coredump user. Proceeding to dump core as root: No such process [ OK ] Stopped Journal Service. And without systemd-journald I couldn't get userspace up and running. A bit of tinkering showed that "vdso32=0" on the kernel command line allows me to get a usable userspace. Any idea where I should look next to pinpoint this? Thanks, Paul Bolle
Re: [PATCH] isdn/gigaset: check endpoint null in gigaset_probe
Phong Tran schreef op vr 26-07-2019 om 20:35 [+0700]: > This fixed the potential reference NULL pointer while using variable > endpoint. > > Reported-by: syzbot+35b1c403a14f5c89e...@syzkaller.appspotmail.com > Tested by syzbot: > https://groups.google.com/d/msg/syzkaller-bugs/wnHG8eRNWEA/Qn2HhjNdBgAJ > > Signed-off-by: Phong Tran > --- > drivers/isdn/gigaset/usb-gigaset.c | 9 + This is now drivers/staging/isdn/gigaset/usb-gigaset.c. > 1 file changed, 9 insertions(+) > > diff --git a/drivers/isdn/gigaset/usb-gigaset.c > b/drivers/isdn/gigaset/usb-gigaset.c > index 1b9b43659bdf..2e011f3db59e 100644 > --- a/drivers/isdn/gigaset/usb-gigaset.c > +++ b/drivers/isdn/gigaset/usb-gigaset.c > @@ -703,6 +703,10 @@ static int gigaset_probe(struct usb_interface *interface, > usb_set_intfdata(interface, cs); > > endpoint = &hostif->endpoint[0].desc; > +if (!endpoint) { > + dev_err(cs->dev, "Couldn't get control endpoint\n"); > + return -ENODEV; > + } When can this happen? Is this one of those bugs that one can only trigger with a specially crafted (evil) usb device? > buffer_size = le16_to_cpu(endpoint->wMaxPacketSize); > ucs->bulk_out_size = buffer_size; > @@ -722,6 +726,11 @@ static int gigaset_probe(struct usb_interface *interface, > } > > endpoint = &hostif->endpoint[1].desc; > +if (!endpoint) { > + dev_err(cs->dev, "Endpoint not available\n"); > + retval = -ENODEV; > + goto error; > + } > > ucs->busy = 0; > Please note that I'm very close to getting cut off from the ISDN network, so the chances of being able to testi this on a live system are getting small. Thanks, Paul Bolle
Re: [Intel-gfx] screen freeze with 5.2-rc6 Dell XPS-13 skylake i915
Hi Jose, James Bottomley schreef op do 18-07-2019 om 06:29 [+0900]: > On Wed, 2019-07-17 at 23:27 +0200, Paul Bolle wrote: > > I've just reached a day of uptime with your revert. (The proper > > uptime is just a fraction of a day, this being a laptop.) Anyhow, no > > screen freezes occurred during this day. > > I'm afraid my status is that I'm in Tokyo doing conference > presentations (and kernel demos) so I'm a bit reluctant to mess with my > setup until I finish everything on Friday, but I will test it after > then, promise. By now I'm testing this for a week (currently on top of 5.2.2). Still no freezes whatsoever. So what's the status of this revert? Unless this is something pretty obscure that for some odd reason only James and I are able to hit it would be nice to get this into stable before the main distros switch over to 5.2.y. Thanks, Paul Bolle
Re: CONFIG_* symbols in UAPI headers?
Paul Bolle schreef op di 09-04-2019 om 21:11 [+0200]: > Does it still apply? It does, cleanly. Output is (for v5.1-rc4): ./usr/include/asm-generic/fcntl.h:119: leaks CONFIG_64BIT to userspace where it is not valid ./usr/include/asm-generic/mman-common.h:22: leaks CONFIG_MMAP_ALLOW_UNINITIALIZED to userspace where it is not valid ./usr/include/linux/elfcore.h:62: leaks CONFIG_BINFMT_ELF_FDPIC to userspace where it is not valid ./usr/include/linux/flat.h:17: leaks CONFIG_BINFMT_SHARED_FLAT to userspace where it is not valid ./usr/include/linux/atmdev.h:104: leaks CONFIG_COMPAT to userspace where it is not valid ./usr/include/linux/pktcdvd.h:37: leaks CONFIG_CDROM_PKTCDVD_WCACHE to userspace where it is not valid ./usr/include/linux/raw.h:17: leaks CONFIG_MAX_RAW_DEVS to userspace where it is not valid ./usr/include/linux/hw_breakpoint.h:27: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS to userspace where it is not valid ./usr/include/linux/eventpoll.h:82: leaks CONFIG_PM_SLEEP to userspace where it is not valid ./usr/include/asm/auxvec.h:14: leaks CONFIG_IA32_EMULATION to userspace where it is not valid ./usr/include/asm/mman.h:7: leaks CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS to userspace where it is not valid Which suggests it might be doable to silence these warnings. Paul Bolle
Re: CONFIG_* symbols in UAPI headers?
Christoph Hellwig schreef op ma 08-04-2019 om 14:46 [+0200]: > There are a few similar issues, like struct elf_prstatus having > a different layout depending on CONFIG_BINFMT_ELF_FDPIC, or > MAX_SHARED_LIBS defending on CONFIG_BINFMT_SHARED_FLAT. I've had the patch pasted below in a local branch for over five years, but never dared to push it upstream. Does it still apply? Thanks, Paul Bolle -- >8 --- >From 0f73c8ee776c197e3029c4eed21af0f121a8f9d3 Mon Sep 17 00:00:00 2001 From: Paul Bolle Date: Tue, 4 Feb 2014 22:22:48 +0100 Subject: [PATCH] headers_check: enable check for CONFIG_ leakage The check for leaked CONFIG_ symbols was disabled in v2.6.30, because it generated too much noise. But a (rather simplistic) preprocessing of comments suffices to silence the noise (ie, no false positives are reported anymore). So add some preprocessing of comments and enable check_config() again. Signed-off-by: Paul Bolle --- scripts/headers_check.pl | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl index b6aec5e4365f..8e67017c1b67 100755 --- a/scripts/headers_check.pl +++ b/scripts/headers_check.pl @@ -36,13 +36,36 @@ foreach my $file (@files) { open(my $fh, '<', $filename) or die "$filename: $!\n"; $lineno = 0; + my $in_comment = 0; + my $swap_state = 0; while ($line = <$fh>) { $lineno++; - &check_include(); - &check_asm_types(); - &check_sizetypes(); - &check_declarations(); - # Dropped for now. Too much noise &check_config(); + + # strip inline comments + $line =~ s|/\*.*\*/||; + + # try to handle multi line comments + if ($in_comment == 0 and $line =~ m|/\*|) { + $line =~ s|/\*.*$||; + # we still need to check (the first half of) this line + # so we set $in_comment after the checks + $swap_state = 1; + } + if ($in_comment == 1 and $line =~ m|\*/|) { + $line =~ s|^.*\*/||; + $in_comment = 0; + } + unless ($in_comment) { + check_include(); + check_asm_types(); + check_sizetypes(); + check_declarations(); + check_config(); + } + if ($swap_state) { + $in_comment = 1; + $swap_state = 0; + } } close $fh; } -- 2.17.2
Re: [PATCH] ser_gigaset: mark expected switch fall-through
Gustavo A. R. Silva schreef op ma 11-02-2019 om 16:34 [-0600]: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. > > This patch fixes the following warning: > > drivers/isdn/gigaset/ser-gigaset.c: In function ‘gigaset_tty_ioctl’: > drivers/isdn/gigaset/ser-gigaset.c:627:3: warning: this statement may fall > through [-Wimplicit-fallthrough=] >switch (arg) { >^~ > drivers/isdn/gigaset/ser-gigaset.c:638:2: note: here > default: > ^~~ > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > Notice that, in this particular case, the code comment is modified > in accordance with what GCC is expecting to find. > > This patch is part of the ongoing efforts to enable > -Wimplicit-fallthrough. > > Signed-off-by: Gustavo A. R. Silva Acked-by: Paul Bolle Thanks, Paul Bolle
Re: Linux 5.0-rc1
Linus Torvalds schreef op zo 06-01-2019 om 18:14 [-0800]: > Nothing particular stands out, although I do like > seeing how some ancient drivers are getting put out to pasture > (*cought*isdn*cough*). Just to let people know: the gigaset drivers will get my palliative care until a few weeks before September 1, 2019. Because at that date the Dutch consumer grade ISDN network (the "BRI" part of ISDN) will be shut down. So expect a patch to remove me from MAINTAINERS sometime during the v5.2 cycle. That leaves just Karsten Keil to look after all the ISDN drivers. Karsten's last activity was in 2016. See commit 1e1589ad8b5c ("mISDN: Support DR6 indication in mISDNipac driver"). Which is also the last commit apparently resolving an issue noticed by an actual _user_ of one of the ISDN drivers. Perhaps Karsten could tell us whether there's any point in keeping the ISDN subsystem in the tree after September 1, 2019, and if so, in what form. I'm in no position to properly answer that question. Paul Bolle
Re: [PATCH 38/42] isdn: replace ->proc_fops with ->proc_show
Hi Christoph, (I don't think the patches of this series ever hit the ISDN related addresses still found in MAINTAINERS. And now I might be a bit late.) Christoph Hellwig schreef op wo 16-05-2018 om 11:43 [+0200]: > diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c > index ccec7778cad2..dac5cd35e901 100644 > --- a/drivers/isdn/gigaset/capi.c > +++ b/drivers/isdn/gigaset/capi.c > @@ -2437,19 +2437,6 @@ static int gigaset_proc_show(struct seq_file *m, void > *v) > return 0; > } > > -static int gigaset_proc_open(struct inode *inode, struct file *file) > -{ > - return single_open(file, gigaset_proc_show, PDE_DATA(inode)); > -} > - > -static const struct file_operations gigaset_proc_fops = { > - .owner = THIS_MODULE, > - .open = gigaset_proc_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release= single_release, > -}; > - > /** > * gigaset_isdn_regdev() - register device to LL > * @cs: device descriptor structure. > @@ -2478,8 +2465,7 @@ int gigaset_isdn_regdev(struct cardstate *cs, const > char *isdnid) > iif->ctr.register_appl = gigaset_register_appl; > iif->ctr.release_appl = gigaset_release_appl; > iif->ctr.send_message = gigaset_send_message; > - iif->ctr.procinfo = gigaset_procinfo; Is this intentional? You didn't touch the procinfo method in the other ISDN drivers, as far as I can see. (If it was intentional, gigaset_procinfo() can of course be removed.) > - iif->ctr.proc_fops = &gigaset_proc_fops; > + iif->ctr.proc_show = gigaset_proc_show, > INIT_LIST_HEAD(&iif->appls); > skb_queue_head_init(&iif->sendqueue); > atomic_set(&iif->sendqlen, 0); Thanks, Paul Bolle
Re: [PATCH] kconfig: Warn if help text is blank
On Tue, 2018-01-30 at 20:33 +0100, Ulf Magnusson wrote: > I agree that this shouldn't go in until/unless a significant portion > of those empty help texts get removed first. Not a significant portion, but all, I'd say. > The patchset that removes the empty help texts is at > https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd > prefer that. I'm fine with anything that doesn't add warnings just to appease pet peeves. > I haven't even checked how they are rendered to be honest. I was more > concerned with the Kconfig mess. Well, if you could make empty help texts disappear somehow (either during the Kconfig parse phase - which requires a jump into the yacc horror - or during the rendering phase) that would be much better than adding a warning for something that you yourself described as a pet peeve. And after that's done you're free to send cleanup patches for this whenever you feel like doing so. Thanks, Paul Bolle
Re: [PATCH] kconfig: Warn if help text is blank
On Tue, 2018-01-30 at 19:18 +0100, Ulf Magnusson wrote: > Print a warning if a 'help' token is given but the help text is blank. > Personal pet peeve. > > Example warnings: > > net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank > help text > net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank > help text > net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank > help text > drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with > blank help text > drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with > blank help text > drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with > blank help text > drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' > defined with blank help text > lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with > blank help text > > A separate patchset will be sent to fix all current instances of blank > help texts for all arches. I added the same warning to Kconfiglib. If you do this it would be better to first fix or remove those help texts, and only then add this warning. Ie, add the warning in the last patch of a cleanup series. > --- a/scripts/kconfig/zconf.y > +++ b/scripts/kconfig/zconf.y > @@ -436,6 +436,12 @@ help: help_start T_HELPTEXT > zconfprint("warning: '%s' defined with more than one help text > -- only the last one will be used", > current_entry->sym->name ?: ""); > } > + > + /* Is the help text empty or all whitespace? */ > + if ($2[strspn($2, " \f\n\r\t\v")] == '\0') > + zconfprint("warning: '%s' defined with blank help text", > +current_entry->sym->name ?: ""); > + Does this go to stderr? Another fix would be to ignore empty help texts and not render them at all. Is that possible? Thanks, Paul Bolle
Re: [PATCH v6 05/99] xarray: Add definition of struct xarray
Mathhew, Just a minor question. On Wed, 2018-01-17 at 12:20 -0800, Matthew Wilcox wrote: > This is a direct replacement for struct radix_tree_root. Some of the > struct members have changed name; convert those, and use a #define so > that radix_tree users continue to work without change. > > Signed-off-by: Matthew Wilcox > --- a/include/linux/xarray.h > +++ b/include/linux/xarray.h > @@ -10,6 +10,8 @@ > */ > > #include > +#include > +#include The top Makefile includes linux/kconfig.h globally. (See the odd USERINCLUDE variable, which is actually part of the LINUXINCLUDE variable, but split off to make things confusing.) Why do you need to include linux/kconfig.h here? > #include > #include Thanks, Paul Bolle
Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
[CC-ing Linus because I quote him.] On Fri, 2017-10-20 at 00:28 +0200, Thomas Gleixner wrote: > Well, that does not explain why > > drivers->cs + i > > would be corrupted. That would require that this cs -> urb link points at > driver magically and then wreckages that driver data structure. Might be > the case, but if so then there are dragons burried somehwere Let's assume dragons are buried somewhere. We need users to show us that they met a dragon, right? (I care little about dragons no-one ever stumbles upon.) In the explanation of commit 9f5af546e6ac ("isdn/i4l: fix buffer overflow") Linus added: [ ISDN seems to be effectively unmaintained, and the I4L driver in particular is long deprecated, but in case somebody uses this.. - Linus ] ISDN is pretty niche. So it's no surprise that in mainline it's divided into three parts: I4L, CAPI, and mISDN. Arnd Bergmann has suggested more than once to move I4L to staging. (As far as I know, moving drivers to staging effectively means removing those drivers, but anyhow.) I'd say we'd just should do that. The stuff has been deemed deprecated since basically forever. I never cared about mISDN, but as far as I can see mISDN has quietly left mainline. The only actively maintained CAPI drivers are gigaset's drivers. But I'm afraid maintaining gigaset basically means seeing treewide cleanups fly by and keeping the various fuzzers happy. I don't mind, and I could keep on doing that for years. But still, I'd love to hear someone say: yes, I still care about mainline ISDN. Does that person still exists? Thanks, Paul Bolle
Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
On Thu, 2017-10-19 at 14:31 -0700, Kees Cook wrote: > What I did in many other non-trivial conversions was just add an > explicit pointer back, since that's operationally identical to what > struct timer_list was storing in its .data field. > > i.e. > > add: > > struct cardstate *cs; > > to struct bas_cardstate, and then use this on timer entry: > >struct bas_cardstate *ucs = from_timer(ucs, t, $timer...); >struct cardstate *cs = ucs->cs; That crossed my mind too. (Honestly!) It _feels_ a bit dirty, as I have this idea that structures having references to each other is some sort of an anti- pattern. On the other hand: the various structures used here are, well, not that clean already so I might as well ignore my feelings. > and this at init: > > spin_lock_init(&ucs->lock); > + ucs->cs = cs; > - setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs); > - setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs); > - setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs); > - setup_timer(&ucs->timer_int_in, int_in_resubmit, (unsigned long) cs); > + timer_setup(&ucs->timer_ctrl, req_timeout, 0); > + timer_setup(&ucs->timer_atrdy, atrdy_timeout, 0); > + timer_setup(&ucs->timer_cmd_in, cmd_in_timeout, 0); > + timer_setup(&ucs->timer_int_in, int_in_resubmit, 0); > > which will avoid the urb entirely. > > Do you want me to send an alternative patch? That would be nice! Please allow a few days for testing. That testing is beyond silly, though. It requires me getting a laptop very close to the awkward place where my ISDN setup lives. I'll spare you the details. But that silliness again shows, I'd say, that the gigaset code mainly exists to see if there's still a pulse in mainline ISDN. Is that enough to bother? Or should mainline ISDN go the way of, say, IRDA? But I digress. An alternative patch would be much appreciated. Thanks, Paul Bolle
Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
On Thu, 2017-10-19 at 23:31 +0200, Thomas Gleixner wrote: > bas_gigaset_exit() > { > for (i = 0; i < driver->minors; i++) { > if (gigaset_shutdown(driver->cs + i) < 0) > > gigaset_shutdown(cs) > { > mutex_lock(&cs->mutex); < Explodes here > > So driver->cs + i is invalid. No idea how that might be related to that > timer conversion patch, but Thanks for peeking into this! Please note that driver->minors is one of the more embarrassing warts of the gigaset code. It's basically hardcoded to 1 for all three drivers (including bas_gigaset). So driver->cs itself is invalid here. And since the patch uses struct cardstate *cs = urb->context; in a few places my guess is that it's really the patch that triggers this. Thanks, Paul Bolle
Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote: > On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote: > > In preparation for unconditionally passing the struct timer_list pointer to > > all timer callbacks, switch to using the new timer_setup() and from_timer() > > to pass the timer pointer explicitly. > > Acked-by: Paul Bolle I have to take this back, sorry! > For the record: this patch made me nervous but survived the rigorous testing I > threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in > just over an hour. Whoot! That's more than good enough to ack this patch.) > > There was some cleanup I had in mind to make this patch more straightforward. > But that can wait until someone finds a way to hit an issue with this patch. > We'll see. That someone turns out to be me, doing "modprobe -r bas_gigaset": <1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 01e9 <1>[30143.538154] IP: mutex_lock+0x19/0x30 <6>[30143.538157] *pde = <5>[30143.538162] Oops: 0002 [#1] SMP <5>[30143.538165] Modules linked in: bas_gigaset(OE-) gigaset vfat fat uas usb_storage ppp_deflate bsd_comp ppp_synctty ppp_generic slhc capi kernelcapi fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_raw iptable_security ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_intel8x0 snd_ac97_codec gpio_ich ac97_bus ppdev iTCO_wdt iTCO_vendor_support lpc_ich snd_seq snd_seq_device snd_pcm pcspkr i2c_i801 thinkpad_acpi <5>[30143.538228] snd_timer snd irda(C) soundcore parport_pc rfkill parport acpi_cpufreq i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops sdhci_pci drm tg3 sdhci mmc_core ata_generic serio_raw yenta_socket ptp pata_acpi pps_core video [last unloaded: gigaset] <0>[30143.538257] CPU: 0 PID: 22085 Comm: modprobe Tainted: G C OE 4.14.0-0.rc4.1.local0.fc26.i686 #1 <0>[30143.538260] Hardware name: IBM 2525FAG/2525FAG, BIOS 74ET64WW (2.09 ) 12/14/2006 <0>[30143.538263] task: f6671100 task.stack: c77c6000 <5>[30143.538267] EIP: mutex_lock+0x19/0x30 <5>[30143.538269] EFLAGS: 00010246 CPU: 0 <5>[30143.538272] EAX: EBX: 01e9 ECX: 0001 EDX: f6671100 <5>[30143.538275] ESI: 01e9 EDI: 011c8f98 EBP: c77c7ef4 ESP: c77c7ef0 <5>[30143.538278] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 <5>[30143.538281] CR0: 80050033 CR2: 01e9 CR3: 16d2d000 CR4: 06d0 <0>[30143.538284] Call Trace: <0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset] <0>[30143.538307] ? find_module_all+0x62/0x80 <0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset] <0>[30143.538319] SyS_delete_module+0x19c/0x240 <0>[30143.538325] ? fput+0xd/0x10 <0>[30143.538330] do_fast_syscall_32+0x71/0x1a0 <0>[30143.538335] entry_SYSENTER_32+0x4e/0x7c <5>[30143.538337] EIP: 0xb7fb5cd9 <5>[30143.538339] EFLAGS: 0206 CPU: 0 <5>[30143.538342] EAX: ffda EBX: 011c8fd4 ECX: 0800 EDX: 011c8fd4 <5>[30143.538345] ESI: 011c8f98 EDI: 011c8f98 EBP: 011c8fd4 ESP: bf8278f8 <5>[30143.538348] DS: 007b ES: 007b FS: GS: 0033 SS: 007b <0>[30143.538351] Code: 8e fb ff ff 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 3e 8d 74 26 00 55 89 e5 53 89 c3 e8 60 e7 ff ff 64 8b 15 40 c9 82 c7 31 c0 <3e> 0f b1 13 85 c0 74 07 89 d8 e8 b8 ff ff ff 5b 5d c3 90 8d 74 <0>[30143.538399] EIP: mutex_lock+0x19/0x30 SS:ESP: 0068:c77c7ef0 <5>[30143.538402] CR2: 01e9 <4>[30143.538406] ---[ end trace 3e60af64adfe7e14 ]--- I'll have to ask for some patience so I can find out what's going on. (Very likely: using urb->context beyond it's lifetime.) Expect something by early next week. Is that OK with you? Sorry for the noise, Paul Bolle
Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote: > In preparation for unconditionally passing the struct timer_list pointer to > all timer callbacks, switch to using the new timer_setup() and from_timer() > to pass the timer pointer explicitly. Acked-by: Paul Bolle For the record: this patch made me nervous but survived the rigorous testing I threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in just over an hour. Whoot! That's more than good enough to ack this patch.) There was some cleanup I had in mind to make this patch more straightforward. But that can wait until someone finds a way to hit an issue with this patch. We'll see. Thanks, Paul Bolle
Re: [PATCH 31/58] isdn/gigaset: Use kzalloc instead of open-coded field zeroing
On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote: > This replaces a kmalloc followed by a bunch of per-field zeroing with a > single kzalloc call, reducing the lines of code. Acked-by: Paul Bolle Thanks, Paul Bolle
Re: [PATCH v2] isdn/gigaset: Convert timers to use timer_setup()
On Thu, 2017-10-05 at 12:31 -0700, Kees Cook wrote: > --- a/drivers/isdn/gigaset/bas-gigaset.c > +++ b/drivers/isdn/gigaset/bas-gigaset.c > -static void cmd_in_timeout(unsigned long data) > +static void cmd_in_timeout(struct timer_list *t) > { > - struct cardstate *cs = (struct cardstate *) data; > - struct bas_cardstate *ucs = cs->hw.bas; > + struct bas_cardstate *ucs = from_timer(ucs, t, timer_cmd_in); > + struct urb *urb = ucs->urb_int_in; > + struct cardstate *cs = urb->context; This makes me nervous. Are you sure urb->context points to a struct cardstate here and in the other two places this patch changes? Anyhow, I'd like to have some time to do my review. So what's your timeframe here? I do hope I have at least a few weeks. (In other words: I hope gigaset isn't the only driver where the ability to use random pointers in these timer callbacks is removed.) Thanks, Paul Bolle
Re: [PATCH] isdn/gigaset: Convert timers to use timer_setup()
Hi Kees, On Wed, 2017-10-04 at 17:52 -0700, Kees Cook wrote: > Also uses kzmalloc to replace open-coded field assignments to NULL and zero. If I'm allowed to whine (chances that I'm allowed to do that are not so great as Dave tends to apply gigaset patches before I even have a chance to look at them properly!): I'd prefer it if that was done separately in a preceding patch. Would that bother you? Thanks, Paul Bolle
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others()
On Fri, 2017-06-09 at 14:32 -0400, Steven Rostedt wrote: > I'm sure it works, but it just adds one more way of doing the same > thing. I thought that was what perl was always criticized for, and why > people usually prefer python. Don't get me wrong, I prefer oysters over > snakes. But I just wanted to point out the lack of consistency here. A major benefit is that #if IS_ENABLED(CONFIG_HYPERV) is shorter than #if defined(CONFIG_HYPERV) || defined(CONFIG_HYPERV_MODULE) and less prone to typos. Paul Bolle
Re: [PATCH 20/35] drivers/isdn: Convert remaining uses of pr_warning to pr_warn
On Thu, 2017-02-16 at 23:11 -0800, Joe Perches wrote: > To enable eventual removal of pr_warning > > This makes pr_warn use consistent for drivers/isdn > > Prior to this patch, there were 20 uses of pr_warning and > 3 uses of pr_warn in drivers/isdn > > Signed-off-by: Joe Perches > --- > drivers/isdn/gigaset/interface.c| 2 +- Gigaset hunk looks good to me: Acked-by: Paul Bolle > drivers/isdn/hardware/mISDN/avmfritz.c | 17 + > drivers/isdn/hardware/mISDN/hfcmulti.c | 8 > drivers/isdn/hardware/mISDN/hfcpci.c| 4 ++-- > drivers/isdn/hardware/mISDN/hfcsusb.c | 4 ++-- > drivers/isdn/hardware/mISDN/mISDNipac.c | 4 ++-- > drivers/isdn/hardware/mISDN/mISDNisar.c | 10 +- > drivers/isdn/hardware/mISDN/netjet.c| 8 > drivers/isdn/hardware/mISDN/w6692.c | 12 ++-- > drivers/isdn/mISDN/hwchannel.c | 8 > 10 files changed, 39 insertions(+), 38 deletions(-) Paul Bolle
Re: [PATCH v5 5/8] Makefile.headersinst: remove destination-y option
On Thu, 2017-02-02 at 14:25 +0100, Nicolas Dichtel wrote: > This option was added in commit c7bb349e7c25 ("kbuild: introduce destination-y > for exported headers") but never used in-tree. > > Signed-off-by: Nicolas Dichtel I've got an identical patch in my ever growing stack of stuff that I should actually submit: Acked-by: Paul Bolle Thanks, Paul Bolle
Re: [PATCH 2/2] isdn: i4l: move active-isdn drivers to staging
On Tue, 2017-01-03 at 23:25 +0100, Arnd Bergmann wrote: > As far as I'm concerned, we are totally fine as long as there exists a > longterm supported kernel that has i4l in drivers/staging. Or in drivers/isdn, right? Paul Bolle
Re: [PATCH 2/2] isdn: i4l: move active-isdn drivers to staging
On Tue, 2017-01-03 at 22:19 +0100, Arnd Bergmann wrote: > Sounds good to me. My original series contained four more patches that > I did not post again after there was some concern[1] that we did not > come to a conclusion on: > > isdn: gigaset: remove i4l code Let me repeat that I'm fine with a patch that does that. > isdn: move isdnhdlc out of i4l > isdn: i4l: move hisax driver to staging > isdn: move i4l to staging > > I can post those as well, at least I think the first two are helpful > for untangling i4l from the rest of ISDN. I also still think that > moving hisax and i4l to staging is reasonable given the state of > that code, even if there are a couple of users today. There are? And even if there are: is there any reason to expect that moving the rest of i4l to staging will result in anything other than a stream of checkpatch cleanups? How often did a bunch of drivers re-enter the tree after being sent to staging? Paul Bolle
Re: [PATCH 2/2] net: wireless: fix to uses struct
On Thu, 2016-12-22 at 01:50 +0300, Ozgur Karatas wrote: > I don't have a problem with C programming I'm sorry, but you do need to learn C, at a basic level, first. Paul Bolle
Re: [PATCH 2/2] net: wireless: fix to uses struct
On Thu, 2016-12-22 at 00:23 +0200, Ozgur Karatas wrote: > I compiled and didn't to errors. Really? $ make net/wireless/reg.o CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALLscripts/checksyscalls.sh CC net/wireless/reg.o net/wireless/reg.c: In function ‘regulatory_hint_core’: net/wireless/reg.c:2294:28: error: ‘regulatory_request’ undeclared (first use in this function) request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL); ^~ net/wireless/reg.c:2294:28: note: each undeclared identifier is reported only once for each function it appears in net/wireless/reg.c: In function ‘regulatory_hint_user’: net/wireless/reg.c:2316:28: error: ‘regulatory_request’ undeclared (first use in this function) request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL); ^~ net/wireless/reg.c: In function ‘regulatory_hint’: net/wireless/reg.c:2388:28: error: ‘regulatory_request’ undeclared (first use in this function) request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL); ^~ scripts/Makefile.build:293: recipe for target 'net/wireless/reg.o' failed make[1]: *** [net/wireless/reg.o] Error 1 Makefile:1640: recipe for target 'net/wireless/reg.o' failed make: *** [net/wireless/reg.o] Error 2 Didn't Thomas Gleixner suggest that you do a basic C course just yesterday? Paul Bolle
[PATCH] checkpatch: stop worrying about include/asm/
include/asm/ got removed in v1.1.45. Two decades later it's about time to stop worrying whether patches still touch it. Signed-off-by: Paul Bolle --- Lightly tested. scripts/checkpatch.pl | 5 - 1 file changed, 5 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a8368d1c4348..4975f251ba80 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2333,11 +2333,6 @@ sub process { WARN("PATCH_PREFIX", "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); } - - if ($realfile =~ m@^include/asm/@) { - ERROR("MODIFIED_INCLUDE_ASM", - "do not modify files in include/asm, change architecture specific files in include/asm-\n" . "$here$rawline\n"); - } $found_file = 1; } -- 2.7.4
Re: Odd build breakage in 4.9-rc7
On Thu, 2016-12-01 at 12:42 -0500, Nicolas Pitre wrote: > OK I understand what the problem is. However most of those hunks below > are definitely wrong. ;-) Probably. By now I've narrowed it down to just these two hunks: diff --git a/scripts/Makefile b/scripts/Makefile index 1d80897a9644..f23e5c4f2496 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -40,7 +40,9 @@ build_docproc: $(obj)/docproc build_check-lc_ctype: $(obj)/check-lc_ctype @: -subdir-$(CONFIG_MODVERSIONS) += genksyms +ifeq ($(or $(CONFIG_MODVERSIONS),$(CONFIG_TRIM_UNUSED_KSYMS)),y) +subdir-y += genksyms +endif subdir-y += mod subdir-$(CONFIG_SECURITY_SELINUX) += selinux subdir-$(CONFIG_DTC) += dtc diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh index 8dc1918b6783..7525da1cc2f7 100755 --- a/scripts/adjust_autoksyms.sh +++ b/scripts/adjust_autoksyms.sh @@ -68,7 +68,7 @@ while read sym; do done >> "$new_ksyms_file" # Special case for modversions (see modpost.c) -if [ -n "$CONFIG_MODVERSIONS" ]; then +if [ -n "$CONFIG_MODVERSIONS" -o -n "$CONFIG_TRIM_UNUSED_KSYMS" ]; then echo "#define __KSYM_module_layout 1" >> "$new_ksyms_file" fi > I'm trying to determine the best way to fix it. Stay tuned. Will do. I'm curious to see what a proper fix might look like. Thanks, Paul Bolle
Re: Odd build breakage in 4.9-rc7
On Thu, 2016-12-01 at 10:01 +0100, Paul Bolle wrote: > Perhaps this is all documented somewhere. But even then it would be nice if > the build would fail right at the start. Ie, the build probably should fail if > one does "make bzImage" while having a .config with > CONFIG_TRIM_UNUSED_KSYMS=y > CONFIG_MODULES=y > # CONFIG_MODVERSIONS is not set > > Because it seems in that case the subsequent "make modules" will then end in > this flood of ERRORs. Or, alternatively, we could use something like the following hack to keep a two step build (ie, "make bzImage" and "make modules") do the right thing even if CONFING_MODVERSIONS is not set. The hack was cobbled together with a fair amount of cargo-cult coding so perhaps a hunk or two aren't really needed. We'll see. Paul Bolle diff --git a/Makefile b/Makefile index 694111b43cf8..5820f803ca64 100644 --- a/Makefile +++ b/Makefile @@ -321,7 +321,7 @@ KBUILD_BUILTIN := 1 # make sure the checksums are up to date before we record them. ifeq ($(MAKECMDGOALS),modules) - KBUILD_BUILTIN := $(if $(CONFIG_MODVERSIONS),1) + KBUILD_BUILTIN := $(if $(or $(CONFIG_MODVERSIONS),$(CONFIG_TRIM_UNUSED_KSYMS)),1) endif # If we have "make modules", compile modules diff --git a/scripts/Makefile b/scripts/Makefile index 1d80897a9644..f23e5c4f2496 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -40,7 +40,9 @@ build_docproc: $(obj)/docproc build_check-lc_ctype: $(obj)/check-lc_ctype @: -subdir-$(CONFIG_MODVERSIONS) += genksyms +ifeq ($(or $(CONFIG_MODVERSIONS),$(CONFIG_TRIM_UNUSED_KSYMS)),y) +subdir-y += genksyms +endif subdir-y += mod subdir-$(CONFIG_SECURITY_SELINUX) += selinux subdir-$(CONFIG_DTC) += dtc diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 7675d11ee65e..50ce2cf86b7c 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -182,7 +182,7 @@ $(obj)/%.symtypes : $(src)/%.c FORCE quiet_cmd_cc_o_c = CC $(quiet_modtag) $@ -ifndef CONFIG_MODVERSIONS +ifneq ($(if $(CONFIG_MODVERSIONS),1,$(if $(CONFIG_TRIM_UNUSED_KSYMS),1)),1) cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< else @@ -358,7 +358,7 @@ $(obj)/%.s: $(src)/%.S FORCE quiet_cmd_as_o_S = AS $(quiet_modtag) $@ -ifndef CONFIG_MODVERSIONS +ifneq ($(if $(CONFIG_MODVERSIONS),1,$(if $(CONFIG_TRIM_UNUSED_KSYMS),1)),1) cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< else diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh index 8dc1918b6783..7525da1cc2f7 100755 --- a/scripts/adjust_autoksyms.sh +++ b/scripts/adjust_autoksyms.sh @@ -68,7 +68,7 @@ while read sym; do done >> "$new_ksyms_file" # Special case for modversions (see modpost.c) -if [ -n "$CONFIG_MODVERSIONS" ]; then +if [ -n "$CONFIG_MODVERSIONS" -o -n "$CONFIG_TRIM_UNUSED_KSYMS" ]; then echo "#define __KSYM_module_layout 1" >> "$new_ksyms_file" fi
Re: Odd build breakage in 4.9-rc7
[Added Nicolas, for whom this all might be painfully obvious.] On Wed, 2016-11-30 at 18:40 -0500, Jarod Wilson wrote: > My config had MODVERSIONS set, yes. For the record triggering this flood of ERRORs for undefined symbols that you ran into has been possible ever since TRIM_UNUSED_KSYMS was added to the tree (in v4.7). I just tested that. Perhaps this is all documented somewhere. But even then it would be nice if the build would fail right at the start. Ie, the build probably should fail if one does "make bzImage" while having a .config with CONFIG_TRIM_UNUSED_KSYMS=y CONFIG_MODULES=y # CONFIG_MODVERSIONS is not set Because it seems in that case the subsequent "make modules" will then end in this flood of ERRORs. Paul Bolle
Re: Odd build breakage in 4.9-rc7
On Wed, 2016-11-30 at 22:42 +0100, Paul Bolle wrote: > My current theory is that setting MODVERSIONS, somehow, hides the ERROR spew. > Because that could explain your bisect. Linus' commit turns of MODVERSIONS the > hard way. And, naturally, this theory fails if your .configs never had > MODVERSIONS set in the first place. This theory appears to be correct! It's getting late here so I won't be spending much time on this anymore. Perhaps you fancy looking into all this now. And, maybe, a new day will bring me the courage needed to dive into this. Thanks, Paul Bolle
Re: Odd build breakage in 4.9-rc7
On Wed, 2016-11-30 at 16:35 -0500, Jarod Wilson wrote: > Just to confirm, with CONFIG_TRIM_UNUSED_KSYMS unset, the build behaves > normally, no ERROR spew. And if MODVERSIONS is not set? My current theory is that setting MODVERSIONS, somehow, hides the ERROR spew. Because that could explain your bisect. Linus' commit turns of MODVERSIONS the hard way. And, naturally, this theory fails if your .configs never had MODVERSIONS set in the first place. I'm testing all this right now, but you probably command more powerful machines and could beat me in testing this theory. Thanks, Paul Bolle
Re: Odd build breakage in 4.9-rc7
On Wed, 2016-11-30 at 12:24 -0500, Jarod Wilson wrote: > Up second, once we're past the above, building modules goes splat: > > 8< > $ make -s ARCH=x86_64 V=1 -j8 modules > ... > ERROR: "module_put" [virt/lib/irqbypass.ko] undefined! > ERROR: "mutex_unlock" [virt/lib/irqbypass.ko] undefined! > ERROR: "mutex_lock" [virt/lib/irqbypass.ko] undefined! > ... > 8< > > There are similar ERROR lines to the tune of 145k lines of output, > basically for every single module and symbol in the build. This breakage > was bisected to commit cd3caefb4663e3811d37cc2afad3cce642d60061, which > looks fairly innocuous, but when reverted, builds work fine again. I ran into a modules build printing over 100K ERROR lines a month ago: https://lkml.kernel.org/r/<1478165881-9263-1-git-send-email-pebo...@tiscali.nl> That had to do with setting TRIM_UNUSED_KSYMS and so unsetting UNUSED_SYMBOLS, as far as I could tell. Did you perhaps also have UNUSED_SYMBOLS unset when your modules build when splat? And did your bzImage build by any chance print this (to stderr): sed: can't read .tmp_versions/*.mod: No such file or directory If so I might have run into your second issue a month ago already, which makes your bisect to commit cd3caefb4663 ("Fix subtle CONFIG_MODVERSIONS problems") suspect. Or did that bisect not cover the second issue? > Multi-threaded make vs. single-threaded doesn't matter, setting > CONFIG_BROKEN=y or '# CONFIG_MODVERSIONS is not set' don't make a > difference, and interestingly, if instead of split 'make bzImage' and > 'make modules', I just do a single 'make', then things DO build > successfully, so I'm a wee bit baffled as to what's actually going on > here. Likewise (ie, both the modules splat going away if doing a single make and being baffled, but more that a wee bit). Paul Bolle
Re: [PATCH] Scripts: kconfig: nconf: fix _GNU_SOURCE redefined warning
[Dropped Yann.] On Tue, 2016-11-29 at 02:46 +0800, Cheah Kok Cheong wrote: > On Mon, Nov 28, 2016 at 11:39:01AM +0100, Michal Marek wrote: > > The Makefile does not specify -D_GNU_SOURCE. Are you adding it manually? On Fedora 24 I'm able to trigger this too: $ [...] make -f ./scripts/Makefile.build obj=scripts/kconfig nconfig gcc -Wp,-MD,scripts/kconfig/.nconf.o.d -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu89 -D_GNU_SOURCE -DCURSES_LOC="" -DNCURSES_WIDECHAR=1 -DLOCALE -c -o scripts/kconfig/nconf.o scripts/kconfig/nconf.c scripts/kconfig/nconf.c:8:0: warning: "_GNU_SOURCE" redefined #define _GNU_SOURCE :0:0: note: this is the location of the previous definition It's probably added by pkg-config: $ pkg-config --cflags ncursesw -D_GNU_SOURCE And apparently we call pkg-config in scripts/kconfig/Makefile. Or in scripts/kconfig/lxdialog/check-lxdialog.sh. Not sure, I didn't yet dive deeper into this. Hope this helps, Paul Bolle
[tip:x86/build] x86/build: Remove three unneeded genhdr-y entries
Commit-ID: 9190e21780dfeff524a67c6e7b806c8a9d496086 Gitweb: http://git.kernel.org/tip/9190e21780dfeff524a67c6e7b806c8a9d496086 Author: Paul Bolle AuthorDate: Fri, 25 Nov 2016 13:41:47 +0100 Committer: Ingo Molnar CommitDate: Mon, 28 Nov 2016 07:49:17 +0100 x86/build: Remove three unneeded genhdr-y entries In x86's include/asm/Kbuild three entries are appended to the genhdr-y make variable: genhdr-y += unistd_32.h genhdr-y += unistd_64.h genhdr-y += unistd_x32.h The same entries are also appended to that variable in include/uapi/asm/Kbuild. So commit: 10b63956fce7 ("UAPI: Plumb the UAPI Kbuilds into the user header installation and checking") ... removed these three entries from include/asm/Kbuild. But, apparently, some merge conflict resolution re-added them. The net effect is, in short, that the genhdr-y make variable contains these file names twice and, as a consequence, that the corresponding headers get installed twice. And so the build prints: INSTALL usr/include/asm/ (65 files) ... while in reality only 62 files are installed in that directory. Nothing breaks because of all that, but it's a good idea to finally remove these unneeded entries nevertheless. Signed-off-by: Paul Bolle Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1480077707-2837-1-git-send-email-pebo...@tiscali.nl Signed-off-by: Ingo Molnar --- arch/x86/include/asm/Kbuild | 4 1 file changed, 4 deletions(-) diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild index 2cfed17..2b892e2 100644 --- a/arch/x86/include/asm/Kbuild +++ b/arch/x86/include/asm/Kbuild @@ -6,10 +6,6 @@ generated-y += unistd_32_ia32.h generated-y += unistd_64_x32.h generated-y += xen-hypercalls.h -genhdr-y += unistd_32.h -genhdr-y += unistd_64.h -genhdr-y += unistd_x32.h - generic-y += clkdev.h generic-y += cputime.h generic-y += dma-contiguous.h
[tip:x86/build] x86/build: Don't use $(LINUXINCLUDE) twice
Commit-ID: 06cbbac0f57d947656a12c30a0a69d4cf0ac6dea Gitweb: http://git.kernel.org/tip/06cbbac0f57d947656a12c30a0a69d4cf0ac6dea Author: Paul Bolle AuthorDate: Fri, 25 Nov 2016 13:38:34 +0100 Committer: Ingo Molnar CommitDate: Mon, 28 Nov 2016 07:49:17 +0100 x86/build: Don't use $(LINUXINCLUDE) twice The make variable KBUILD_CFLAGS contains $(LINUXINCLUDE). But the build already picks up $(LINUXINCLUDE) from scripts/Makefile.lib. The net effect is that the (long) list of include directories is used twice. This is harmless but pointless. So stop using $(LINUXINCLUDE) twice. Signed-off-by: Paul Bolle Cc: Linus Torvalds Cc: Masahiro Yamada Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1480077514-2586-1-git-send-email-pebo...@tiscali.nl Signed-off-by: Ingo Molnar --- arch/x86/realmode/rm/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile index 25012ab..4463fa7 100644 --- a/arch/x86/realmode/rm/Makefile +++ b/arch/x86/realmode/rm/Makefile @@ -69,7 +69,7 @@ $(obj)/realmode.relocs: $(obj)/realmode.elf FORCE # --- -KBUILD_CFLAGS := $(LINUXINCLUDE) $(REALMODE_CFLAGS) -D_SETUP -D_WAKEUP \ +KBUILD_CFLAGS := $(REALMODE_CFLAGS) -D_SETUP -D_WAKEUP \ -I$(srctree)/arch/x86/boot KBUILD_AFLAGS := $(KBUILD_CFLAGS) -D__ASSEMBLY__ GCOV_PROFILE := n
[PATCH] x86/build: remove three unneeded genhdr-y entries
In x86's include/asm/Kbuild three entries are appended to the genhdr-y make variable: genhdr-y += unistd_32.h genhdr-y += unistd_64.h genhdr-y += unistd_x32.h The same entries are also appended to that variable in include/uapi/asm/Kbuild. So commit 10b63956fce7 ("UAPI: Plumb the UAPI Kbuilds into the user header installation and checking") removed these three entries from include/asm/Kbuild. But, apparently, some merge conflict resolution re-added them. The net effect is, in short, that the genhdr-y make variable contains these file names twice and, as a consequence, that the corresponding headers get installed twice. And so the build prints INSTALL usr/include/asm/ (65 files) while in reality only 62 files are installed in that directory. Nothing breaks because of all that, but it's a good idea to finally remove these unneeded entries nevertheless. Signed-off-by: Paul Bolle --- arch/x86/include/asm/Kbuild | 4 1 file changed, 4 deletions(-) diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild index 2cfed174e3c9..2b892e2313a9 100644 --- a/arch/x86/include/asm/Kbuild +++ b/arch/x86/include/asm/Kbuild @@ -6,10 +6,6 @@ generated-y += unistd_32_ia32.h generated-y += unistd_64_x32.h generated-y += xen-hypercalls.h -genhdr-y += unistd_32.h -genhdr-y += unistd_64.h -genhdr-y += unistd_x32.h - generic-y += clkdev.h generic-y += cputime.h generic-y += dma-contiguous.h -- 2.7.4
[PATCH] x86/realmode: Don't use $(LINUXINCLUDE) twice
The make variable KBUILD_CFLAGS contains $(LINUXINCLUDE). But the build already picks up $(LINUXINCLUDE) from scripts/Makefile.lib. The net effect is that the (long) list of include directories is used twice. This is harmless but pointless. So stop using $(LINUXINCLUDE) twice. Signed-off-by: Paul Bolle --- arch/x86/realmode/rm/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile index 25012abc3409..4463fa72db94 100644 --- a/arch/x86/realmode/rm/Makefile +++ b/arch/x86/realmode/rm/Makefile @@ -69,7 +69,7 @@ $(obj)/realmode.relocs: $(obj)/realmode.elf FORCE # --- -KBUILD_CFLAGS := $(LINUXINCLUDE) $(REALMODE_CFLAGS) -D_SETUP -D_WAKEUP \ +KBUILD_CFLAGS := $(REALMODE_CFLAGS) -D_SETUP -D_WAKEUP \ -I$(srctree)/arch/x86/boot KBUILD_AFLAGS := $(KBUILD_CFLAGS) -D__ASSEMBLY__ GCOV_PROFILE := n -- 2.7.4
[PATCH] MIPS: ZBOOT: Don't use $(LINUXINCLUDE) twice
The make variables KBUILD_CFLAGS and KBUILD_AFLAGS both contain $(LINUXINCLUDE). But the build already picks up $(LINUXINCLUDE) from scripts/Makefile.lib. The net effect is that the (long) list of include directories is used twice. This is harmless but pointless. So stop using $(LINUXINCLUDE) twice. Signed-off-by: Paul Bolle --- Build tested only! arch/mips/boot/compressed/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile index 011b14512c05..41564a4db6f7 100644 --- a/arch/mips/boot/compressed/Makefile +++ b/arch/mips/boot/compressed/Makefile @@ -22,10 +22,10 @@ KBUILD_CFLAGS := $(shell echo $(KBUILD_CFLAGS) | sed -e "s/-pg//") KBUILD_CFLAGS := $(filter-out -fstack-protector, $(KBUILD_CFLAGS)) -KBUILD_CFLAGS := $(LINUXINCLUDE) $(KBUILD_CFLAGS) -D__KERNEL__ \ +KBUILD_CFLAGS := $(KBUILD_CFLAGS) -D__KERNEL__ \ -DBOOT_HEAP_SIZE=$(BOOT_HEAP_SIZE) -D"VMLINUX_LOAD_ADDRESS_ULL=$(VMLINUX_LOAD_ADDRESS)ull" -KBUILD_AFLAGS := $(LINUXINCLUDE) $(KBUILD_AFLAGS) -D__ASSEMBLY__ \ +KBUILD_AFLAGS := $(KBUILD_AFLAGS) -D__ASSEMBLY__ \ -DBOOT_HEAP_SIZE=$(BOOT_HEAP_SIZE) \ -DKERNEL_ENTRY=$(VMLINUX_ENTRY_ADDRESS) -- 2.7.4
[PATCH] cris: No need to append -O2 and $(LINUXINCLUDE)
The make variables asflags-y and ccflags-y are appended with -O2 and $(LINUXINCLUDE). But the build already picks up -O2 from the top Makefile and $(LINUXINCLUDE) from scripts/Makefile.lib. The net effect is that -O2 and the (long) list of include directories are used twice. This is harmless but pointless. So stop appending to these flags. Signed-off-by: Paul Bolle --- Build tested only! arch/cris/boot/compressed/Makefile | 3 --- arch/cris/boot/rescue/Makefile | 3 --- 2 files changed, 6 deletions(-) diff --git a/arch/cris/boot/compressed/Makefile b/arch/cris/boot/compressed/Makefile index 8fe9338c1775..e4ba0be0e782 100644 --- a/arch/cris/boot/compressed/Makefile +++ b/arch/cris/boot/compressed/Makefile @@ -2,9 +2,6 @@ # arch/cris/boot/compressed/Makefile # -asflags-y += $(LINUXINCLUDE) -ccflags-y += -O2 $(LINUXINCLUDE) - # asflags-$(CONFIG_ETRAX_ARCH_V32) += -I$(srctree)/include/asm/mach \ # -I$(srctree)/include/asm/arch # ccflags-$(CONFIG_ETRAX_ARCH_V32) += -O2 -I$(srctree)/include/asm/mach diff --git a/arch/cris/boot/rescue/Makefile b/arch/cris/boot/rescue/Makefile index d98edbb30a18..a82025940006 100644 --- a/arch/cris/boot/rescue/Makefile +++ b/arch/cris/boot/rescue/Makefile @@ -8,9 +8,6 @@ # asflags-y += -I $(srctree)/include/asm/arch/mach/ -I $(srctree)/include/asm/arch # LD = gcc-cris -mlinux -march=v32 -nostdlib -asflags-y += $(LINUXINCLUDE) -ccflags-y += -O2 $(LINUXINCLUDE) - ifdef CONFIG_ETRAX_AXISFLASHMAP arch-$(CONFIG_ETRAX_ARCH_V10) = v10 -- 2.7.4
Re: Linux 4.9: Reported regressions as of Sunday, 2016-11-20
On Sun, 2016-11-20 at 16:09 +0100, Thorsten Leemhuis wrote: > Desc: "build regression: make.cross ARCH=mips fails with ""No rule to make > target 'alchemy/devboards/'. """ > Repo: 16-10-30 > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1262410.html > https://marc.info/?l=linux-kernel&m=147780880425626 > Stat: n/a > Note: nothing happened yet; BTW: Should build regressions be on this list at > all? Fixed by commit 818f38c5b7c4 ("MIPS: Fix build of compressed image"). So this is actually fixed since v4.9-rc4, but I only looked into this again today. Thanks, Paul Bolle
Re: i2c: undefined option I2C_ALGO_BUSCLEAR
Hi Valentin, On Thu, 2016-11-17 at 12:33 +0100, Valentin Rothberg wrote: > I tested your patch and it works properly for me. It even still applies > on linux-next. Cool. (We should probably compile a list of current invalid selects somewhere publicly, and team up to submit patches for the low hanging fruit (ie, obvious typos) and for the arches that actually matter before I resubmit.) > Would it be possible to extend your patch to also check > symbols in other statements (depends on, if, etc.)? No. Depending on an unknown symbol is common and correct. Think depends on SOME_OBSCURE_ARM_BOARD somewhere in drivers. Only arm builds will ever know about the SOME_OBSCURE_ARM_BOARD symbol. What could be done is starting any "make *config" target with a treewide pass over all Kconfig files to collect all valid symbols and so spot typos and other obviously incorrect. The example that comes to mind is depends on SH that I have spotted a few times in the past years. But this treewide pass will incur some runtime cost and might not be easy to implement cleanly. Perhaps we're better of with using your script for that. Thanks, Paul Bolle
Re: i2c: undefined option I2C_ALGO_BUSCLEAR
[Added Michal] Hi Valentin, On Wed, 2016-11-16 at 08:31 +0100, Valentin Rothberg wrote: > your commit c3ca951fe41a ("i2c: Add Tegra BPMP I2C proxy driver") > popped up in today's linux-next tree, adding Kconfig option > I2C_TEGRA_BPMP, which further selects I2C_ALGO_BUSCLEAR, which is > nowhere defined in Kconfig. > > Is there a patch queued somewhere to add I2C_ALGO_BUSCLEAR to Kconfig? > I could not find anything on the lkml; only some older repositories > on github, where the options is defined in drivers/i2c/busses/Kconfig. Attached is a v2 of a patch I submitted way to long ago. It adds a warning for exactly this issue. That should help others to spot them. (It's a bit smarter than your check kconfig script as it also warns if you select symbols that are outside of you arch's scope. Ie, not only for entirely unknown symbols.) Please play with it. Unless you spot some issues I really should be properly resubmitting it. Thanks, Paul Bolle >8 >8 >8 >8 --- From: Paul Bolle Subject: [PATCH] kconfig: warn if an unknown symbol is selected A select of an unknown symbol is basically treated as a nop and is silently skipped. This is annoying if the selected symbol contains a typo. It can also hide the fact that a treewide symbol cleanup was only done partially. There are also a few cases were this might have been done on purpose. But that anti-pattern should be discouraged. Almost all select statements point to a known and reachable symbol. So people will likely assume that any selected symbol is actually set. Selects that violate this assumption can only be spotted by checking multiple Kconfig files, often across architectures. It is unlikely that people will do this regularily. So let's warn when we notice a select of a symbol that is not known in the configuration we're creating. Signed-off-by: Paul Bolle Acked-by: Michal Marek (v1) --- scripts/kconfig/conf.c | 42 ++ 1 file changed, 42 insertions(+) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 866369f10ff8..55a4dacb196d 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -447,6 +447,46 @@ static void check_conf(struct menu *menu) check_conf(child); } +static void check_selects(struct menu *menu) +{ + struct symbol *sym, *sel; + struct property *prop; + + while (menu) { + sym = menu->sym; + + if (sym && !sym_is_choice(sym) && + sym->type != S_UNKNOWN && + sym_get_tristate_value(sym) != no && + sym->flags & SYMBOL_WRITE) { + for_all_properties(sym, prop, P_SELECT) { + sel = prop->expr->left.sym; + if (sel->type == S_UNKNOWN && + expr_calc_value(prop->visible.expr) != no) { + fprintf(stderr, "%s:%d:warning: ", + prop->file->name, + prop->lineno); + fprintf(stderr, + "'%s' selects unknown symbol '%s'\n", + sym->name, + sel->name); + } + } + } + + if (menu->list) { + menu = menu->list; + } else if (menu->next) { + menu = menu->next; + } else while ((menu = menu->parent)) { + if (menu->next) { + menu = menu->next; + break; + } + } + } +} + static struct option long_opts[] = { {"oldaskconfig",no_argument, NULL, oldaskconfig}, {"oldconfig", no_argument, NULL, oldconfig}, @@ -686,6 +726,8 @@ int main(int ac, char **av) break; } + check_selects(rootmenu.list); + if (sync_kconfig) { /* silentoldconfig is used during the build so we shall update autoconf. * All other commands are only used to generate a config. -- 2.7.4
Re: [PATCH] [media] gp8psk-fe: add missing MODULE_foo() macros
On Mon, 2016-11-14 at 11:14 -0200, Mauro Carvalho Chehab wrote: > --- a/drivers/media/dvb-frontends/gp8psk-fe.c > +++ b/drivers/media/dvb-frontends/gp8psk-fe.c > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the Free > + * Software Foundation, version 2. > +MODULE_LICENSE("GPL"); Nit: according to the comments in include/linux/module.h the "ident" that matches the license described at the top of this file is "GPL v2". Paul Bolle
Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c
On Mon, 2016-11-07 at 14:38 +0100, Heiko Carstens wrote: > On Mon, Nov 07, 2016 at 02:13:06PM +0100, Paul Bolle wrote: > > --- /dev/null > > +++ b/arch/s390/include/asm/facilities.h > > @@ -0,0 +1,43 @@ > > +#ifndef __ASM_FACILITIES_H > > +#define __ASM_FACILITIES_H > > + > > +#define FACILITIES_ALS \ > > + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 0 | /* N3 > > instructions */ \ > > So, this is wrong. It should be " << 63" to match the existing code. Bother. > > +#define FACILITIES_KVM \ > > + _BITUL(0) |/* N3 instructions */ \ > > + _BITUL(1) |/* z/Arch mode installed */ \ > > + _BITUL(2) |/* z/Arch mode active */ \ > > + _BITUL(3) |/* DAT-enhancement */ \ > > + _BITUL(4) |/* idte segment table */ \ > > + _BITUL(5) |/* idte region table */ \ > > + _BITUL(6) |/* ASN-and-LX reuse */ \ > > + _BITUL(7) |/* stfle */ \ > > + _BITUL(8) |/* enhanced-DAT 1 */ \ > > + _BITUL(9) |/* sense-running-status */ \ > > + _BITUL(10) |/* conditional sske */ \ > > + _BITUL(13) |/* ipte-range */ \ > > + _BITUL(14) /* nonquiescing key-setting */ \ > > + , \ > > + _BITUL(9) |/* transactional execution */ \ > > + _BITUL(11) |/* access-exception-fetch/store indication */ \ > > And this is exactly what I want to avoid: start counting from zero again if > we cross a double word. It _must_ read 73, 75, otherwise this becomes the > unmaintainable and error prone mess we had before. > > I just want a list with bit numbers and the rest must be created > automatically. This sounds like a can of worms. Better keep it closed. Thanks, Paul Bolle
Re: [PATCH 1/2] s390: delete unneeded #include from facilities_src.h
Hi Mashiro, On Tue, 2016-11-08 at 10:50 +0900, Masahiro Yamada wrote: > 2016-11-07 21:52 GMT+09:00 Paul Bolle : > > So it seems the odd $(LINUXINCLUDE) variable in that Makefile could be > > replaced with something like: > > -include $(srctree)/include/generated/autoconf.h > > This would break O= build because autoconf.h is a generated file. > > Rather, it should be > -include $(objtree)/include/generated/autoconf.h Three cheers for weasel words like "something like"! > I thought of this at first, but I was not quite sure > if the file path include/generated/autoconf.h is a guaranteed interface. > > Basically, now we are supposed to include autoconf.h via kconfig.h. Yes, that seems to go back to commit 2a11c8ea20bf ("kconfig: Introduce IS_ENABLED(), IS_BUILTIN() and IS_MODULE()"). And when the current approach to the IS_*() macros was introduced - with that breathtaking hack that introduced __is_defined() - this was no longer needed but was not changed again. > So, I thought $(LINUXINCLUDE) is a more stable interface > than specifying the exact path to autoconf.h > > I doubt that nobody would try to change it, but it is just two my cents. A bit of cruft accumulated around LINUXINCLUDE: a few dubious uses of it (and I think this is one of those); typos (ie, LINUX_INCLUDE); the pointless USERINCLUDE; things like that. It would be nice to remove that cruft. But it needs to be done carefully. > Anyway, arch/x86/boot/Makefile already > referenced the path to autoconf.h > > So, if you want to change it, I will not oppose to it. Paul Bolle
Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c
On Mon, 2016-11-07 at 10:50 +0100, Martin Schwidefsky wrote: > Heiko Carstens wrote: > > On Sun, Nov 06, 2016 at 12:45:28PM +0900, Masahiro Yamada wrote: > > > > > > We generally expect headers in arch/$(ARCH)/include/asm directory > > > are included from kernel sources, but facilities_src.h is not; > > > it is included from the arch/s390/tools/gen_facilities.c tool. > > > > > > There is no reason to expose this header to the public include path. > > > Furthermore, facilities_src.h makes sure to be included only from > > > gen_facilities.c by the following: > > > > > > #ifndef S390_GEN_FACILITIES_C > > > #error "This file can only be included by gen_facilities.c" > > > #endif > > > > > > This check can be removed by merging the two files. > > > > > > Signed-off-by: Masahiro Yamada It took me some time to figure out that gen_facilities is only used to generate a small header file (generated/facilities.h). And that header's only goal is to define FACILITIES_ALS and FACILITIES_KVM. Pasted below is an attempt to use asm/facilities.h instead of generated/facilities.h. That allows to drop arch/s390/tools/ entirely. I don't actually have an s390 machine at hand to test this, but this does build and the preprocessed code this generates looks sane. (Yes, asm/facilities.h might need another level of preprocessor defines to become actually readable.) Thanks, Paul Bolle diff --git a/arch/s390/Makefile b/arch/s390/Makefile index 54e00526b8df..a0ee0a1ee677 100644 --- a/arch/s390/Makefile +++ b/arch/s390/Makefile @@ -108,7 +108,6 @@ drivers-y += drivers/s390/ drivers-$(CONFIG_OPROFILE) += arch/s390/oprofile/ boot := arch/s390/boot -tools := arch/s390/tools all: image bzImage @@ -127,10 +126,6 @@ vdso_install: archclean: $(Q)$(MAKE) $(clean)=$(boot) - $(Q)$(MAKE) $(clean)=$(tools) - -archprepare: - $(Q)$(MAKE) $(build)=$(tools) include/generated/facilities.h # Don't use tabs in echo arguments define archhelp diff --git a/arch/s390/include/asm/facilities.h b/arch/s390/include/asm/facilities.h new file mode 100644 index ..c87f18d29217 --- /dev/null +++ b/arch/s390/include/asm/facilities.h @@ -0,0 +1,43 @@ +#ifndef __ASM_FACILITIES_H +#define __ASM_FACILITIES_H + +#define FACILITIES_ALS \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 0 | /* N3 instructions */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 1 | /* z/Arch mode installed */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 7 | /* stfle */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 17 | /* message security assist */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z990_FEATURES), UL) << 18 |/* long displacement facility */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 21 | /* extended-immediate facility */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 25 | /* store clock fast */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 27 | /* mvcos */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 32 | /* compare and swap and store */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 33 | /* compare and swap and store 2 */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 34 | /* general extension facility */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 35 | /* execute extensions */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z196_FEATURES), UL) << 45 |/* fast-BCR, etc. */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_ZEC12_FEATURES), UL) << 49 | /* misc-instruction-extensions */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_ZEC12_FEATURES), UL) << 52 | /* interlocked facility 2 */ \ + _AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z13_FEATURES), UL) << 53 /* load-and-zero-rightmost-byte, etc. */ + +#define FACILITIES_KVM \ + _BITUL(0) |/* N3 instructions */ \ + _BITUL(1) |/* z/Arch mode installed */ \ + _BITUL(2) |/* z/Arch mode active */ \ + _BITUL(3) |/* DAT-enhancement */ \ + _BITUL(4) |/* idte segment table */ \ + _BITUL(5) |/* idte region table */ \ + _BITUL(6) |/* ASN-and-LX reuse */ \ + _BITUL(7) |/* stfle */ \ + _BITUL(8) |/* enhanced-DAT 1 */ \ + _BITUL(9) |/* sense-running-status */ \ + _BITUL(10) |/* conditional sske */ \ + _BITUL(13) |/* ipte-range */ \ + _BITUL(14) /* nonquiescing key-setting */ \ + , \ + _BITUL(9) |/* transactional execution */ \ + _BITUL(11) |/* access-e
Re: [PATCH 1/2] s390: delete unneeded #include from facilities_src.h
On Sun, 2016-11-06 at 12:45 +0900, Masahiro Yamada wrote: > The header facilities_src.h is only included from gen_facilities.c > and the tool is compiled with the following extra options: > > HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE) > > Please note $(LINUXINCLUDE) is expanded into build options including: > > -include $(srctree)/include/linux/kconfig.h > > So, the Makefile always forces the tool to include kconfig.h, i.e., > the #include directive in the header is redundant. As far as I can see the only kernel header that gen_facilities.c is actually interested in is autoconf.h. (autoconf.h will be included via in kconfig.h.) So it seems the odd $(LINUXINCLUDE) variable in that Makefile could be replaced with something like: -include $(srctree)/include/generated/autoconf.h Paul Bolle
[tip:x86/boot] x86/boot/build: Remove always empty $(USERINCLUDE)
Commit-ID: 0acba3f91823a5e53a54af5dc31fc774b0e64e99 Gitweb: http://git.kernel.org/tip/0acba3f91823a5e53a54af5dc31fc774b0e64e99 Author: Paul Bolle AuthorDate: Thu, 3 Nov 2016 10:47:48 +0100 Committer: Ingo Molnar CommitDate: Mon, 7 Nov 2016 07:30:01 +0100 x86/boot/build: Remove always empty $(USERINCLUDE) Commmit b6eea87fc685: ("x86, boot: Explicitly include autoconf.h for hostprogs") correctly noted: [...] that because $(USERINCLUDE) isn't exported by the top-level Makefile it's actually empty in arch/x86/boot/Makefile. So let's do the sane thing and remove the reference to that make variable. Signed-off-by: Paul Bolle Reviewed-by: Matt Fleming Cc: David Howells Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1478166468-9760-1-git-send-email-pebo...@tiscali.nl Signed-off-by: Ingo Molnar --- arch/x86/boot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile index 12ea8f8..0d810fb 100644 --- a/arch/x86/boot/Makefile +++ b/arch/x86/boot/Makefile @@ -65,7 +65,7 @@ clean-files += cpustr.h # --- -KBUILD_CFLAGS := $(USERINCLUDE) $(REALMODE_CFLAGS) -D_SETUP +KBUILD_CFLAGS := $(REALMODE_CFLAGS) -D_SETUP KBUILD_AFLAGS := $(KBUILD_CFLAGS) -D__ASSEMBLY__ GCOV_PROFILE := n UBSAN_SANITIZE := n
Re: [PATCH] x86/boot: Remove always empty $(USERINCLUDE)
Apparently Matt left Intel. Let's forward this to a recently used address. On Thu, 2016-11-03 at 10:47 +0100, Paul Bolle wrote: > Commmit b6eea87fc685 ("x86, boot: Explicitly include autoconf.h for > hostprogs") correctly noted > [...] that because $(USERINCLUDE) isn't exported by > the top-level Makefile it's actually empty in arch/x86/boot/Makefile. > > So let's do the sane thing and remove the reference to that make variable. > > Signed-off-by: Paul Bolle > --- > arch/x86/boot/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile > index 12ea8f8384f4..0d810fb15eac 100644 > --- a/arch/x86/boot/Makefile > +++ b/arch/x86/boot/Makefile > @@ -65,7 +65,7 @@ clean-files += cpustr.h > > # --- > > -KBUILD_CFLAGS:= $(USERINCLUDE) $(REALMODE_CFLAGS) -D_SETUP > +KBUILD_CFLAGS:= $(REALMODE_CFLAGS) -D_SETUP > KBUILD_AFLAGS:= $(KBUILD_CFLAGS) -D__ASSEMBLY__ > GCOV_PROFILE := n > UBSAN_SANITIZE := n
[PATCH] x86/boot: Remove always empty $(USERINCLUDE)
Commmit b6eea87fc685 ("x86, boot: Explicitly include autoconf.h for hostprogs") correctly noted [...] that because $(USERINCLUDE) isn't exported by the top-level Makefile it's actually empty in arch/x86/boot/Makefile. So let's do the sane thing and remove the reference to that make variable. Signed-off-by: Paul Bolle --- arch/x86/boot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile index 12ea8f8384f4..0d810fb15eac 100644 --- a/arch/x86/boot/Makefile +++ b/arch/x86/boot/Makefile @@ -65,7 +65,7 @@ clean-files += cpustr.h # --- -KBUILD_CFLAGS := $(USERINCLUDE) $(REALMODE_CFLAGS) -D_SETUP +KBUILD_CFLAGS := $(REALMODE_CFLAGS) -D_SETUP KBUILD_AFLAGS := $(KBUILD_CFLAGS) -D__ASSEMBLY__ GCOV_PROFILE := n UBSAN_SANITIZE := n -- 2.7.4
[PATCH] [media] dvb-usb: remove another redundant #include
Kernel source files need not include explicitly because the top Makefile forces to include it with: -include $(srctree)/include/linux/kconfig.h Remove another reduntdant include, that managed to sneak by commit 97139d4a6f26 ("treewide: remove redundant #include "). Signed-off-by: Paul Bolle --- drivers/media/usb/dvb-usb/dibusb-mc-common.c | 1 - 1 file changed, 1 deletion(-) Masahiro: another #include was added to include/asm-generic/export.h. I think it's bogus too, but testing it required setting CONFIG_TRIM_UNUSED_KSYMS. And doing that triggered over 100K (!) undefined symbol errors in my test build. So naturally I decided to behave as if I never saw that #include enter export.h. Perhaps you are braver than I am. diff --git a/drivers/media/usb/dvb-usb/dibusb-mc-common.c b/drivers/media/usb/dvb-usb/dibusb-mc-common.c index d66f56cc46a5..c989cac9343d 100644 --- a/drivers/media/usb/dvb-usb/dibusb-mc-common.c +++ b/drivers/media/usb/dvb-usb/dibusb-mc-common.c @@ -9,7 +9,6 @@ * see Documentation/dvb/README.dvb-usb for more information */ -#include #include "dibusb.h" /* 3000MC/P stuff */ -- 2.7.4
Re: [RFC PATCH] get_maintainer: Look for arbitrary letter prefixes in sections
On Thu, 2016-11-03 at 02:16 -0700, Joe Perches wrote: > Yes, it's been reported and should be fixed in -mm. > The fix should show up in -next in a little bit. Great. > For now, try: > $ sed -i -e 's/\xA0/ /g' scripts/get_maintainer.pl Thanks, Paul Bolle
Re: [RFC PATCH] get_maintainer: Look for arbitrary letter prefixes in sections
On Mon, 2016-10-24 at 11:05 -0700, Joe Perches wrote: > Jani Nikula proposes patches to add a few new letter prefixes > for "B:" bug reporting and "C:" maintainer chatting to the > various sections of MAINTAINERS. > > Add a generic mechanism to get_maintainer.pl to find sections that > have any combination of "[A-Z]" letter prefix types in a section. > > Signed-off-by: Joe Perches This patch made it into linux-next (ie, next-20161028). > --- a/scripts/get_maintainer.pl > +++ b/scripts/get_maintainer.pl > @@ -271,7 +273,8 @@ $output_multiline = 0 if ($output_separator ne ", "); > $output_rolestats = 1 if ($interactive); > $output_roles = 1 if ($output_rolestats); > > -if ($sections) { > +if ($sections || $letters ne "") { > +$sections = 1; This triggers: Unrecognized character \xA0; marked by <-- HERE after <-- HERE near column 1 at ./scripts/get_maintainer.pl line 277. Git blame shows: git blame -L 277,+1 ./scripts/get_maintainer.pl b67071653d3fc (Joe Perches 2016-10-28 13:22:01 +1100 277) $sections = 1; (A0 seems to be the no break space. That character was inserted more often further down the patch.) Anybody else seeing this? Paul Bolle
[PATCH] [TRIVIAL] Treewide: Remove references to make variable LINUX_INCLUDE
Commit 4fd06960f120 ("Use the new x86 setup code for i386") introduced a reference to the make variable LINUX_INCLUDE. That reference got moved around a bit and copied twice and now there are three references to it. There has never been a definition of that variable. (Presumably that is because it started out as a mistyped reference to LINUXINCLUDE.) So this reference has always been an empty string. Let's remove it before it spreads any further. Signed-off-by: Paul Bolle --- arch/s390/boot/compressed/Makefile| 2 +- arch/x86/boot/compressed/Makefile | 2 +- drivers/firmware/efi/libstub/Makefile | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/s390/boot/compressed/Makefile b/arch/s390/boot/compressed/Makefile index 0daa070d6c9d..6bd2c9022be3 100644 --- a/arch/s390/boot/compressed/Makefile +++ b/arch/s390/boot/compressed/Makefile @@ -10,7 +10,7 @@ targets := vmlinux.lds vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 targets += vmlinux.bin.xz vmlinux.bin.lzma vmlinux.bin.lzo vmlinux.bin.lz4 targets += misc.o piggy.o sizes.h head.o -KBUILD_CFLAGS := -m64 -D__KERNEL__ $(LINUX_INCLUDE) -O2 +KBUILD_CFLAGS := -m64 -D__KERNEL__ -O2 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING KBUILD_CFLAGS += $(cflags-y) -fno-delete-null-pointer-checks -msoft-float KBUILD_CFLAGS += $(call cc-option,-mpacked-stack) diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 536ccfcc01c6..123e4ad2318a 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -25,7 +25,7 @@ KCOV_INSTRUMENT := n targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \ vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ $(LINUX_INCLUDE) -O2 +KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING cflags-$(CONFIG_X86_32) := -march=i386 diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 5e23e2d305e7..25a86e78683d 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -6,7 +6,7 @@ # cflags-$(CONFIG_X86_32):= -march=i386 cflags-$(CONFIG_X86_64):= -mcmodel=small -cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ $(LINUX_INCLUDE) -O2 \ +cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ -fPIC -fno-strict-aliasing -mno-red-zone \ -mno-mmx -mno-sse -- 2.7.4
[PATCH] [TRIVIAL] Remove last traces of ikconfig.h
The build system stopped generating ikconfig.h in v2.6.8. Remove an entry for it in dontdiff. There's also a reference to it in a small comment. Remove that comment too, as it is of little help in any case. Signed-off-by: Paul Bolle --- Documentation/dontdiff | 1 - kernel/Makefile| 2 -- 2 files changed, 3 deletions(-) diff --git a/Documentation/dontdiff b/Documentation/dontdiff index 5385cba941d2..a23edccd2059 100644 --- a/Documentation/dontdiff +++ b/Documentation/dontdiff @@ -139,7 +139,6 @@ hpet_example hugepage-mmap hugepage-shm ihex2fw -ikconfig.h* inat-tables.c initramfs_list int16.c diff --git a/kernel/Makefile b/kernel/Makefile index eb26e12c6c2a..eaee9de224bd 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -115,8 +115,6 @@ obj-$(CONFIG_HAS_IOMEM) += memremap.o $(obj)/configs.o: $(obj)/config_data.h -# config_data.h contains the same information as ikconfig.h but gzipped. -# Info from config_data can be extracted from /proc/config* targets += config_data.gz $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE $(call if_changed,gzip) -- 2.7.4
Re: Linux 4.9: Reported regressions as of Sunday, 2016-10-30
On Sun, 2016-10-30 at 14:20 +0100, Thorsten Leemhuis wrote: > As always: Are you aware of any other regressions? Then please let me > know (simply CC regressi...@leemhuis.info). Do build regressions count? Because I was trying to fix an obscure build issue in arch/mips, choose a random configuration that should hit that issue, and promptly ran into https://lkml.kernel.org/r/<201610301405.k82kqqw0%25fengguang...@intel.com> The same configuration does build under v4.8, I tested that of course. (Side note: I had to manually insert "25" after "%" to get this to work. Should Intel fix its mail setup, or should lkml.kernel.org learn to escape "%"?) Thanks, Paul Bolle
Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
On Tue, 2016-11-01 at 00:48 +0100, Borislav Petkov wrote: > --- a/drivers/lguest/Kconfig > +++ b/drivers/lguest/Kconfig > config LGUEST > tristate "Linux hypervisor example code" > - depends on X86_32 && EVENTFD && TTY && PCI_DIRECT > > + depends on X86_32 && EVENTFD && TTY && PCI_DIRECT && !MICROCODE > select HVC_DRIVER > ---help--- > This is a very simple module which allows you to run LGUEST is the symbol for host support. The symbol for guest support is LGUEST_GUEST and it lives in arch/x86/. Yes, it's a bit of a gotcha. > but maybe the better fix is to hack in MSR emulation in lguest and > intercept the *MSR accesses and do the writes/reads in the exception > fixup and ... > > I haven't looked at the lguest code, of course and whether that's easily > doable and whether it even makes sense and whether one should simply use > qemu/kvm instead and, and, and... Yeah, I thought about adding negative dependencies (eg, "!OLPC && !MICROCODE") too. But that would be contrary to the neat lguest goal to be able to use the same kernel image as a host and a guest. At least, I think that is one of its goals. And as probably everybody capable of hacking on lguest (ie, other people than me) came up with doubts similar to yours, these two issues never got fixed. Thanks, Paul Bolle
Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
On Tue, 2016-11-01 at 00:04 +0100, Borislav Petkov wrote: > Huh, what does that have to do with disabling the microcode loader? > > Funny... See https://lists.ozlabs.org/pipermail/lguest/2013-May/002001.html . Nobody cared enough to fix it. I cared enough to figure it all out. But I didn't understand much of the possible solutions that where suggested three years ago. I'm certain I still ran into this issue with the microcode loader in the beginning of this year, so that was probably with a v4.4 based guest. Paul Bolle
Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
On Mon, 2016-10-31 at 15:18 -0700, Andy Lutomirski wrote: > I havne't meaningfully tested lguest because I can't get it to work > even without these patches. Have you disabled CONFIG_OLPC in the .config for the guest kernel? Otherwise you will run into lguest: Reinjecting trap 13 for fault at 0x162: Invalid argument A similar obscure gotcha (I think it is "unhandled trap 13") can be avoided by launching the guest with the dis_ucode_ldr kernel option. Hope this helps, Paul Bolle
Re: [PATCH] drivers/net/usb/r8152 fix broken rx checksums
On Sun, 2016-10-30 at 17:22 -0400, David Miller wrote: > 3) "Fix broken RX checksums." Commit header lines and commit > messages are proper English, therefore sentences should > begin with a capitalized letter and end with a period. Commit messages should be proper English. But commit header lines should not end with a period. The vast majority doesn't. Yes, I've just checked. How many newspaper headlines end with a period? Thanks, Paul Bolle
Re: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help
On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote: > When POSIX timers are configured out, the PTP clock subsystem should be > left out as well. However a bunch of ethernet drivers currently *select* > the later in their Kconfig entries. Therefore some more work was needed > to break that hard dependency from those drivers without preventing their > usage altogether. By the way: would you have pointers to threads that discussed attempts to achieve this using currently available Kconfig options? Thanks, Paul Bolle
Re: [PATCH v2 1/5] kconfig: introduce the "imply" keyword
On Thu, 2016-10-27 at 23:10 -0400, Nicolas Pitre wrote: > On Fri, 28 Oct 2016, Paul Bolle wrote: > > And in your example BAR is bool, right? Does the above get more > > complicated if BAR would be tristate? > > If BAR=m then implying BAZ from FOO=y will force BAZ to y or n, > bypassing the restriction provided by BAR like "select" does. This is > somewhat questionable for "select" to do that, and the code emits a > warning when "select" bypasses a direct dependency set to n, but not > when set to m. For now "imply" simply tries to be consistent with > the "select" behavior. Side note: yes, one can select a symbol that's missing one or more dependencies. But since Kconfig has two separate methods to describe relations (ie, selecting and depending) there's logically the possibility of conflict. So we need a rule to resolve that conflict. That rule is: "select" beats "depends on". I don't think that this rule is less plausible than the opposite rule. Paul Bolle
Re: [PATCH v2 1/5] kconfig: introduce the "imply" keyword
On Thu, 2016-10-27 at 23:10 -0400, Nicolas Pitre wrote: > On Fri, 28 Oct 2016, Paul Bolle wrote: > > What happens when a tristate symbol is implied by a symbol set to 'y' > > and by a symbol set to 'm'? > > That's respectively the third and second rows in the table above. I meant: two separate symbols implying the same symbol at the same time. One of those symbols set to 'y' and the other set to 'm'. Anyhow, I hope to play with a mock Kconfig fragment the next few days to find out myself. Thanks, Paul Bolle
Re: [PATCH v2 1/5] kconfig: introduce the "imply" keyword
On Thu, 2016-10-27 at 23:10 -0400, Nicolas Pitre wrote: > On Fri, 28 Oct 2016, Paul Bolle wrote: > > You probably got "["if" ]" for free by copying what's there for > > select. But this series doesn't use it, so perhaps it would be better > > to not document it yet. But that is rather sneaky. Dunno. > > If it is not documented then the chance that someone uses it are slim. > And since it comes "for free" I don't see the point of making the tool > less flexible. And not having this flexibility could make some > transitions from "select" to "imply" needlessly difficult. My point is moot. I somehow missed that this series adds imply PTP_1588_CLOCK if TILEGX So you are quite right in documenting this. Paul Bolle
pinctrl: mediatek: build failure if CONFIG_IRQ_DOMAIN is not set
Hi, 0) A rather spartan build, on x86_64, which did include drivers/pinctrl/mediatek/pinctrl-mtk-common.o failed like this: drivers/pinctrl/mediatek/pinctrl-mtk-common.c: In function ‘mtk_gpio_to_irq’: drivers/pinctrl/mediatek/pinctrl-mtk-common.c:838:8: error: implicit declaration of function ‘irq_find_mapping’ [-Werror=implicit-function-declaration] irq = irq_find_mapping(pctl->domain, pin->eint.eintnum); ^~~~ drivers/pinctrl/mediatek/pinctrl-mtk-common.c: In function ‘mtk_pctrl_init’: drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1474:17: error: implicit declaration of function ‘irq_domain_add_linear’ [-Werror=implicit-function-declaration] pctl->domain = irq_domain_add_linear(np, ^ drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1475:27: error: ‘irq_domain_simple_ops’ undeclared (first use in this function) pctl->devdata->ap_num, &irq_domain_simple_ops, NULL); ^ drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1475:27: note: each undeclared identifier is reported only once for each function it appears in drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1484:14: error: implicit declaration of function ‘irq_create_mapping’ [-Werror=implicit-function-declaration] int virq = irq_create_mapping(pctl->domain, i); ^~ cc1: some warnings being treated as errors make[3]: *** [drivers/pinctrl/mediatek/pinctrl-mtk-common.o] Error 1 make[2]: *** [drivers/pinctrl/mediatek] Error 2 make[1]: *** [drivers/pinctrl] Error 2 make: *** [drivers] Error 2 1) That build had CONFIG_COMPILE_TEST set (obviously) but CONFIG_IRQ_DOMAIN not set. 2) This quick hack fixes that for me: diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig index 419ea4d5964d..066087156dcc 100644 --- a/drivers/pinctrl/mediatek/Kconfig +++ b/drivers/pinctrl/mediatek/Kconfig @@ -7,6 +7,7 @@ config PINCTRL_MTK select GENERIC_PINCONF select GPIOLIB select OF_GPIO + select IRQ_DOMAIN # For ARMv7 SoCs config PINCTRL_MT2701 3) Would you like me to submit a proper (but lightly tested) patch or do you prefer to fix this yourself? Thanks, Paul Bolle
Re: [PATCH v2 1/5] kconfig: introduce the "imply" keyword
On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote: > The "imply" keyword is a weak version of "select" where the target > config symbol can still be turned off, avoiding those pitfalls that come > with the "select" keyword. > > This is useful e.g. with multiple drivers that want to indicate their > ability to hook into a given subsystem "hook into a [...] subsystem" is rather vague. > while still being able to configure that subsystem out s/being able to/allowing the user to/, correct? > and keep those drivers selected. Perhaps replace that with: "without also having to unset these drivers"? > Currently, the same effect can almost be achieved with: > > config DRIVER_A > tristate > > config DRIVER_B > tristate > > config DRIVER_C > tristate > > config DRIVER_D > tristate > > [...] > > config SUBSYSTEM_X > tristate > default DRIVER_A || DRIVER_B || DRIVER_C || DRIVER_D || [...] > > This is unwieldly unwieldy > to maintain especially with a large number of drivers. > Furthermore, there is no easy way to restrict the choice for SUBSYSTEM_X > to y or n, excluding m, when some drivers are built-in. The "select" > keyword allows for excluding m, but it excludes n as well. Hence > this "imply" keyword. The above becomes: > > config DRIVER_A > tristate > imply SUBSYSTEM_X > > config DRIVER_B > tristate > imply SUBSYSTEM_X > > [...] > > config SUBSYSTEM_X > tristate > > This is much cleaner, and way more flexible than "select". SUBSYSTEM_X > can still be configured out, and it can be set as a module when none of > the drivers are selected or all of them are also modular. [I already commented on this sentence in a previous message.] > --- a/Documentation/kbuild/kconfig-language.txt > +++ b/Documentation/kbuild/kconfig-language.txt > That will limit the usefulness but on the other hand avoid > the illegal configurations all over. > > +- weak reverse dependencies: "imply" ["if" ] You probably got "["if" ]" for free by copying what's there for select. But this series doesn't use it, so perhaps it would be better to not document it yet. But that is rather sneaky. Dunno. > + This is similar to "select" as it enforces a lower limit on another > + symbol except that the "implied" config symbol's value may still be > + set to n from a direct dependency or with a visible prompt. This is seriously hard to parse. But it's late here, so it might just be me. > + Given the following example: > + > + config FOO > + tristate > + imply BAZ > + > + config BAZ > + tristate > + depends on BAR > + > + The following values are possible: > + > + FOO BAR BAZ's default choice for BAZ > + --- --- - -- > + n y n N/m/y > + m y m M/y/n > + y y y Y/n Also n n * N m n * N Is that right? > + y n * N But what does '*' mean? What happens when a tristate symbol is implied by a symbol set to 'y' and by a symbol set to 'm'? And in your example BAR is bool, right? Does the above get more complicated if BAR would be tristate? How does setting a direct default for BAZ interfere with the implied values? > + This is useful e.g. with multiple drivers that want to indicate their > + ability to hook into a given subsystem while still being able to > + configure that subsystem out and keep those drivers selected. See my comments above. > b) Match dependency semantics: > b1) Swap all "select FOO" to "depends on FOO" or, > b2) Swap all "depends on FOO" to "select FOO" > + c) Consider the use of "imply" instead of "select" If memory serves me right this text was added after a long discussion with Luis Rodriguez. Luis might want to have a look at any Haven't looked at the code yet, sorry. I'm still trying to see whether I can wrap my mind around this. It looks like just setting a default on another symbol, but there could be a twist I missed. Paul Bolle
Re: [PATCH v2 2/5] kconfig: introduce the "suggest" keyword
On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote: > Similar to "imply" but with no added restrictions on the target symbol's > value. Useful for providing a default value to another symbol. > > Suggested by Edward Cree. > > Signed-off-by: Nicolas Pitre As far as I can see this series doesn't add a user of "suggest". So I see no reason to add it. NAK. Paul Bolle
Re: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help
On Wed, 2016-10-26 at 19:41 -0400, Nicolas Pitre wrote: > On Thu, 27 Oct 2016, Paul Bolle wrote: > > If you'll have to send another update don't bother including Yann. Yann > > hasn't be heard of in years. MAINTAINERS doesn't reflect reality for > > kconfig. > > What about updating it then? I don't know why Yann is still mentioned after M: in MAINTAINERS. In practice Michal has (again) been taking care of kconfig. Paul Bolle
Re: [PATCH v2 1/5] kconfig: introduce the "imply" keyword
On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote: > SUBSYSTEM_X can still be configured out, and it can be set as a > module when none of the drivers are selected or all of them are also > modular. Short note, to highlight a pet peeve: "select" (and therefor "selected") has a special meaning in kconfig land. I guess you meant something neutral like "set" here. Is that correct? By the way: "also" makes this sentence hard to parse. Paul Bolle
Re: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help
On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote: > From: Nicolas Pitre > Subject: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help This doesn't work. I received this message with an empty subject. If you'll have to send another update don't bother including Yann. Yann hasn't be heard of in years. MAINTAINERS doesn't reflect reality for kconfig. Paul Bolle
Re: [tip:x86/vdso] x86/arch_prctl/vdso: Add ARCH_MAP_VDSO_*
This is bit late, but we're still on v4.9-rc2, so anyhow. On Wed, 2016-09-14 at 12:34 -0700, tip-bot for Dmitry Safonov wrote: > Commit-ID: 2eefd8789698e89c4a5d610921dc3c1b66e3bd0d > Gitweb: http://git.kernel.org/tip/2eefd8789698e89c4a5d610921dc3c1b66e3bd0d > Author: Dmitry Safonov > AuthorDate: Mon, 5 Sep 2016 16:33:05 +0300 > Committer: Thomas Gleixner > CommitDate: Wed, 14 Sep 2016 21:28:09 +0200 > > x86/arch_prctl/vdso: Add ARCH_MAP_VDSO_* > > Add API to change vdso blob type with arch_prctl. > As this is usefull only by needs of CRIU, expose > this interface under CONFIG_CHECKPOINT_RESTORE. > > Signed-off-by: Dmitry Safonov > Acked-by: Andy Lutomirski > Cc: 0x7f454...@gmail.com > Cc: o...@redhat.com > Cc: linux...@kvack.org > Cc: gorcu...@openvz.org > Cc: xe...@virtuozzo.com > Link: http://lkml.kernel.org/r/20160905133308.28234-4-dsafo...@virtuozzo.com > Signed-off-by: Thomas Gleixner > --- a/arch/x86/include/uapi/asm/prctl.h > +++ b/arch/x86/include/uapi/asm/prctl.h > #define ARCH_GET_FS 0x1003 > #define ARCH_GET_GS 0x1004 > > +#ifdef CONFIG_CHECKPOINT_RESTORE > +# define ARCH_MAP_VDSO_X32 0x2001 > +# define ARCH_MAP_VDSO_320x2002 > +# define ARCH_MAP_VDSO_640x2003 > +#endif > + > #endif /* _ASM_X86_PRCTL_H */ On my machine this header ends up in /usr/include/asm/prctl.h. But in userspace CONFIG_CHECKPOINT_RESTORE is meaningless. I think if you actually want to export these three macros to userspace the guard should read: #if defined(CONFIG_CHECKPOINT_RESTORE) || !defined(__KERNEL__) And if you don't want to export these macros the guard should read: #if defined(CONFIG_CHECKPOINT_RESTORE) && defined(__KERNEL__) (In that case you're probably better of defining these macros outside of uapi.) I've only lightly tested those two alternatives, so please double check. Paul Bolle
[tip:x86/asm] x86/decoder: Use stderr if insn sanity test fails
Commit-ID: bb12d6740f6de393927362f23f833a79d85df384 Gitweb: http://git.kernel.org/tip/bb12d6740f6de393927362f23f833a79d85df384 Author: Paul Bolle AuthorDate: Tue, 25 Oct 2016 22:56:05 +0200 Committer: Ingo Molnar CommitDate: Wed, 26 Oct 2016 08:41:06 +0200 x86/decoder: Use stderr if insn sanity test fails If the instruction sanity test fails, it prints a "Failure" message to stdout. Make this program behave like the rest of the build and print that message to stderr. Signed-off-by: Paul Bolle Cc: Andrew Morton Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1477428965-20548-3-git-send-email-pebo...@tiscali.nl Signed-off-by: Ingo Molnar --- arch/x86/tools/insn_sanity.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/tools/insn_sanity.c b/arch/x86/tools/insn_sanity.c index ba70ff2..1972565 100644 --- a/arch/x86/tools/insn_sanity.c +++ b/arch/x86/tools/insn_sanity.c @@ -269,7 +269,8 @@ int main(int argc, char **argv) insns++; } - fprintf(stdout, "%s: %s: decoded and checked %d %s instructions with %d errors (seed:0x%x)\n", + fprintf((errors) ? stderr : stdout, + "%s: %s: decoded and checked %d %s instructions with %d errors (seed:0x%x)\n", prog, (errors) ? "Failure" : "Success", insns,
[tip:x86/asm] x86/decoder: Use stdout if insn decoder test is successful
Commit-ID: bdcc18b548b8f1fab23c097724c6f32daac03185 Gitweb: http://git.kernel.org/tip/bdcc18b548b8f1fab23c097724c6f32daac03185 Author: Paul Bolle AuthorDate: Tue, 25 Oct 2016 22:56:04 +0200 Committer: Ingo Molnar CommitDate: Wed, 26 Oct 2016 08:41:06 +0200 x86/decoder: Use stdout if insn decoder test is successful If the instruction decoder test ran successful it prints a message like this to stderr: Succeed: decoded and checked 1767380 instructions But, as described in "console mode programming user interface guidelines version 101" which doesn't exist, programs should use stderr for errors or warnings. We're told about a successful run here, so the instruction decoder test should use stdout. Let's fix the typo too, while we're at it. Signed-off-by: Paul Bolle Cc: Andrew Morton Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1477428965-20548-2-git-send-email-pebo...@tiscali.nl Signed-off-by: Ingo Molnar --- arch/x86/tools/test_get_len.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c index 56f04db..ecf31e0 100644 --- a/arch/x86/tools/test_get_len.c +++ b/arch/x86/tools/test_get_len.c @@ -167,7 +167,7 @@ int main(int argc, char **argv) fprintf(stderr, "Warning: decoded and checked %d" " instructions with %d warnings\n", insns, warnings); else - fprintf(stderr, "Succeed: decoded and checked %d" + fprintf(stdout, "Success: decoded and checked %d" " instructions\n", insns); return 0; }
[PATCH 0/2] x86: use stdout for success and stdout for failure
For some time now my x86 builds are warning free. Yay! But there's still one single line printed to stderr: Succeed: decoded and checked 1767380 instructions This message is printed if the "insn decoder test" ran successful. Patch 1/2 sends this message to stdout (and fixes the typo). Patch 2/2 does the reverse for the "insn sanity test": it sends a message to stderr if that test fails. One thing puzzles me about these two tests: I must have run them hundreds of times but I've never seen them fail. Have other people ran into failures? Paul Bolle (2): x86: use stdout if insn decoder test is successful x86: use stderr if insn sanity test fails arch/x86/tools/insn_sanity.c | 3 ++- arch/x86/tools/test_get_len.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) -- 2.7.4
[PATCH 2/2] x86: use stderr if insn sanity test fails
If the instruction sanity test fails, it prints a "Failure" message to stdout. Make this program behave like the rest of the build and print that message to stderr. Signed-off-by: Paul Bolle --- arch/x86/tools/insn_sanity.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/tools/insn_sanity.c b/arch/x86/tools/insn_sanity.c index ba70ff232917..1972565ab106 100644 --- a/arch/x86/tools/insn_sanity.c +++ b/arch/x86/tools/insn_sanity.c @@ -269,7 +269,8 @@ int main(int argc, char **argv) insns++; } - fprintf(stdout, "%s: %s: decoded and checked %d %s instructions with %d errors (seed:0x%x)\n", + fprintf((errors) ? stderr : stdout, + "%s: %s: decoded and checked %d %s instructions with %d errors (seed:0x%x)\n", prog, (errors) ? "Failure" : "Success", insns, -- 2.7.4
[PATCH 1/2] x86: use stdout if insn decoder test is successful
If the instruction decoder test ran successful it prints a message like this to stderr: Succeed: decoded and checked 1767380 instructions But, as described in "console mode programming user interface guidelines version 101" which doesn't exist, programs should use stderr for errors or warnings. We're told about a successful run here, so the instruction decoder test should use stdout. Let's fix the typo too, while we're at it. Signed-off-by: Paul Bolle --- The joke about the guidelines is shamelessly copied from commit 55a6e622e66a ("arch/x86/tools/insn_sanity.c: Identify source of messages"). Let's hope Andrew doesn't mind! arch/x86/tools/test_get_len.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c index 56f04db0c9c0..ecf31e0358c8 100644 --- a/arch/x86/tools/test_get_len.c +++ b/arch/x86/tools/test_get_len.c @@ -167,7 +167,7 @@ int main(int argc, char **argv) fprintf(stderr, "Warning: decoded and checked %d" " instructions with %d warnings\n", insns, warnings); else - fprintf(stderr, "Succeed: decoded and checked %d" + fprintf(stdout, "Success: decoded and checked %d" " instructions\n", insns); return 0; } -- 2.7.4
Re: [Intel-gfx] drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)
[Detailed post, but please give it a quick scan.] On Wed, 2016-10-12 at 14:06 +0200, Paul Bolle wrote: > On Wed, 2016-10-12 at 14:08 +0300, Joonas Lahtinen wrote: > > Bisecting the offending commit between v4.8 and v4.8.1 would be a good > > start. > > That would be between v4.7 and v4.8. (I guess my report was > ambiguous.) > > That might take some time. Because bisecting always takes a long time > and especially since hitting this WARNING sometimes takes over an hour. > Anyhow, please prod me if I stay silent for too long. 0) So I've lost my courage to do a bisect when my first attempt landed me in v4.6-rc3. This is about for issue popping up between v4.7 and v4.8-rc1. 1) So I used the most reliable debugging tool that I actually understand: printk(): diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index fbcfed63a76e..791de414cf1e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14771,10 +14771,16 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state return DRM_PLANE_HELPER_NO_SCALING; crtc_clock = crtc_state->base.adjusted_mode.crtc_clock; - cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk; + if (WARN_ON_ONCE(!crtc_clock)) + return DRM_PLANE_HELPER_NO_SCALING; - if (WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)) + cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk; + if (WARN_ON_ONCE(cdclk < crtc_clock)) { + printk(KERN_DEBUG "i915: cdclk < crtc_clock: %d < %d\n", cdclk, crtc_clock); return DRM_PLANE_HELPER_NO_SCALING; + } + + printk_ratelimited(KERN_DEBUG "i915: cdclk >= crtc_clock: %d >= %d\n", cdclk, crtc_clock); /* * skl max scale is lower of: 2) This taught me that crtc_clock always is 373250 on my machine. cdclk mostly is 45, but every now and then it briefly is 337500. 3) Now the interesting pattern is that cdclk drops to 337500 only after two quick calls of skl_max_scale() with cdclk set to 45, and a roughly 300ms pause before the third call of that function. Example: <7>[23758.501933] i915: cdclk >= crtc_clock: 45 >= 373250 <7>[23758.515211] i915: cdclk >= crtc_clock: 45 >= 373250 <7>[23758.869057] i915: cdclk < crtc_clock: 337500 < 373250 This pattern of cdclk being 337500 after roughly 300ms is surprisingly stable. 4) So _perhaps_ there's some roughly 300ms timeout, somehow, somewhere, that sets cdclk to 337500 and triggers this issue. Ideas? To be continued, Paul Bolle
[PATCH][TRIVIAL] crypto: ccp - fix typo "CPP"
The abbreviation for Cryptographic Coprocessor is "CCP". Signed-off-by: Paul Bolle --- include/linux/ccp.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/ccp.h b/include/linux/ccp.h index a7653339fedb..c71dd8fa5764 100644 --- a/include/linux/ccp.h +++ b/include/linux/ccp.h @@ -11,8 +11,8 @@ * published by the Free Software Foundation. */ -#ifndef __CPP_H__ -#define __CPP_H__ +#ifndef __CCP_H__ +#define __CCP_H__ #include #include @@ -553,7 +553,7 @@ enum ccp_engine { #define CCP_CMD_PASSTHRU_NO_DMA_MAP0x0002 /** - * struct ccp_cmd - CPP operation request + * struct ccp_cmd - CCP operation request * @entry: list element (ccp driver use only) * @work: work element used for callbacks (ccp driver use only) * @ccp: CCP device to be run on (ccp driver use only) -- 2.7.4
Re: [PATCH] kconfig.h: remove config_enabled() macro
[Added Nicolas.] On Thu, 2016-10-20 at 21:06 +0900, Masahiro Yamada wrote: > 2016-10-20 19:04 GMT+09:00 Paul Bolle : > > On Sun, 2016-10-16 at 20:07 +0900, Masahiro Yamada wrote: > > There are about a dozen instances of IS_ENABLED() that target something > > other than a kconfig macros. Are you planning to convert those to > > __is_defined() too? > > I did not notice that, but looks like there are some to be checked. Are you willing to do that or should I give it a try (after this patch has landed, of course)? > > > --- a/include/linux/kconfig.h > > > +++ b/include/linux/kconfig.h > > > @@ -31,7 +31,6 @@ > > > * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and > > > when > > > * the last step cherry picks the 2nd arg, we get a zero. > > > */ > > > -#define config_enabled(cfg) ___is_defined(cfg) > > > > Is there a reason to keep using the double underscore prefix? > > I followed the suggestion in the following message: > > https://lkml.org/lkml/2016/6/6/944 Nicolas: was there any specific reason to suggest __is_defined() and not, say, is_defined()? > > > #define __is_defined(x) ___is_defined(x) > > > #define > > > ___is_defined(val) is_defined(__ARG_PLACEHOLDER_##v > > > al) > > > #define is_defined(arg1_or_junk) > > > __take_second_arg(arg1_or_junk 1, 0) > > > > __is_defined() is now meant to be used generally, and not just on > > kconfig macros. Can it be moved into another header? > > Currently, __is_defined() is only used in two places: > > include/linux/export.h > include/asm-generic/export.h > > Even if we fix something like IS_ENABLED(DEBUG), > we do not have many for now, > but perhaps will it be used more widely in the future? > > If so, do we need to add IS_DEFINED() or is_defined()? Either is fine with me. I'll gladly defer to anyone with good taste in naming things. > in include/linux/kconfig.h ? or include/linux/kernel.h ? kernel.h seems to be included just about anywhere and it contains various preprocessor macros of general utility. So that looks like a fine candidate. Would it be a problem to put it there? Thanks, Paul Bolle
Re: [PATCH] kconfig.h: remove config_enabled() macro
Masahiro, A few comments regarding, I guess, future work. On Sun, 2016-10-16 at 20:07 +0900, Masahiro Yamada wrote: > The use of config_enabled() is ambiguous. For config options, > IS_ENABLED(), IS_REACHABLE(), etc. will make intention clearer. > Sometimes config_enabled() has been used for non-config options > because it is useful to check whether the given symbol is defined > or not. > > I have been tackling on deprecating config_enabled(), and now is the > time to finish this work. > > Some new users have appeared for v4.9-rc1, but it is trivial to > replace them: > > - arch/x86/mm/kaslr.c > replace config_enabled() with IS_ENABLED() because > CONFIG_X86_ESPFIX64 and CONFIG_EFI are boolean. > > - include/asm-generic/export.h > replace config_enabled() with __is_defined(). > > Then, config_enabled() can be removed now. > > Going forward, please use IS_ENABLED(), IS_REACHABLE(), etc. for > config options, and __is_defined() for non-config symbols. There are about a dozen instances of IS_ENABLED() that target something other than a kconfig macros. Are you planning to convert those to __is_defined() too? > Signed-off-by: Masahiro Yamada > --- a/include/linux/kconfig.h > +++ b/include/linux/kconfig.h > @@ -31,7 +31,6 @@ > * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when > * the last step cherry picks the 2nd arg, we get a zero. > */ > -#define config_enabled(cfg) ___is_defined(cfg) Is there a reason to keep using the double underscore prefix? > #define __is_defined(x) ___is_defined(x) > #define ___is_defined(val) is_defined(__ARG_PLACEHOLDER_##val) > #define is_defined(arg1_or_junk) __take_second_arg(arg1_or_junk 1, 0) __is_defined() is now meant to be used generally, and not just on kconfig macros. Can it be moved into another header? Paul Bolle
Re: [PATCH 4.4 0/2] 4.4.26-stable review
On Wed, 2016-10-19 at 21:34 +0200, Greg Kroah-Hartman wrote: > It's in good company, sitting along with 250+ other patches I have yet > to work through to apply to the stable kernels. For various reasons I > needed to get a round of stable kernels out sooner, which is why it > isn't in there. Don't worry, it's not lost, it will get handled > eventually... Great. I'll be patient from now on. Sorry for the noise. Paul Bolle
Re: [PATCH 4.4 0/2] 4.4.26-stable review
On Wed, 2016-10-19 at 20:30 +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.4.26 release. > There are 2 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. Did I botch my attempt at a backport of "lightnvm: ensure that nvm_dev_ops can be used without CONFIG_NVM" to v4.4.y (see https://lkml.kernel.org/r/<1476477349-28155-1-git-send-email-pebolle@ti scali.nl> ) sufficiently for it to be dropped? Thanks, Paul Bolle
Re: [PATCH] lightnvm: ensure that nvm_dev_ops can be used without CONFIG_NVM
On Fri, 2016-10-14 at 22:35 +0200, Paul Bolle wrote: > From: Jens Axboe > > From: Jens Axboe Bother. Resend? Paul Bolle
[PATCH] lightnvm: ensure that nvm_dev_ops can be used without CONFIG_NVM
From: Jens Axboe From: Jens Axboe commit a7fd9a4f3e8179bab31e4637236ebb0e0b7867c6 upstream. null_blk defines an empty version of this ops structure if CONFIG_NVM isn't set, but it doesn't know the type. Move those bits out of the protection of CONFIG_NVM in the main lightnvm include. Signed-off-by: Jens Axboe [pebolle: backport to v4.4] Signed-off-by: Paul Bolle --- Jens, there's a missing include of in v4.4.y, which is making me rather nervous. Please double check my backport. include/linux/lightnvm.h | 121 +-- 1 file changed, 64 insertions(+), 57 deletions(-) diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h index f09648d14694..782d4e814e21 100644 --- a/include/linux/lightnvm.h +++ b/include/linux/lightnvm.h @@ -1,6 +1,8 @@ #ifndef NVM_H #define NVM_H +#include + enum { NVM_IO_OK = 0, NVM_IO_REQUEUE = 1, @@ -11,10 +13,71 @@ enum { NVM_IOTYPE_GC = 1, }; +#define NVM_BLK_BITS (16) +#define NVM_PG_BITS (16) +#define NVM_SEC_BITS (8) +#define NVM_PL_BITS (8) +#define NVM_LUN_BITS (8) +#define NVM_CH_BITS (8) + +struct ppa_addr { + /* Generic structure for all addresses */ + union { + struct { + u64 blk : NVM_BLK_BITS; + u64 pg : NVM_PG_BITS; + u64 sec : NVM_SEC_BITS; + u64 pl : NVM_PL_BITS; + u64 lun : NVM_LUN_BITS; + u64 ch : NVM_CH_BITS; + } g; + + u64 ppa; + }; +}; + +struct nvm_rq; +struct nvm_id; +struct nvm_dev; + +typedef int (nvm_l2p_update_fn)(u64, u32, __le64 *, void *); +typedef int (nvm_bb_update_fn)(struct ppa_addr, int, u8 *, void *); +typedef int (nvm_id_fn)(struct nvm_dev *, struct nvm_id *); +typedef int (nvm_get_l2p_tbl_fn)(struct nvm_dev *, u64, u32, + nvm_l2p_update_fn *, void *); +typedef int (nvm_op_bb_tbl_fn)(struct nvm_dev *, struct ppa_addr, int, + nvm_bb_update_fn *, void *); +typedef int (nvm_op_set_bb_fn)(struct nvm_dev *, struct nvm_rq *, int); +typedef int (nvm_submit_io_fn)(struct nvm_dev *, struct nvm_rq *); +typedef int (nvm_erase_blk_fn)(struct nvm_dev *, struct nvm_rq *); +typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *); +typedef void (nvm_destroy_dma_pool_fn)(void *); +typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t, + dma_addr_t *); +typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t); + +struct nvm_dev_ops { + nvm_id_fn *identity; + nvm_get_l2p_tbl_fn *get_l2p_tbl; + nvm_op_bb_tbl_fn*get_bb_tbl; + nvm_op_set_bb_fn*set_bb_tbl; + + nvm_submit_io_fn*submit_io; + nvm_erase_blk_fn*erase_block; + + nvm_create_dma_pool_fn *create_dma_pool; + nvm_destroy_dma_pool_fn *destroy_dma_pool; + nvm_dev_dma_alloc_fn*dev_dma_alloc; + nvm_dev_dma_free_fn *dev_dma_free; + + unsigned intmax_phys_sect; +}; + + + #ifdef CONFIG_NVM #include -#include #include #include @@ -126,29 +189,6 @@ struct nvm_tgt_instance { #define NVM_VERSION_MINOR 0 #define NVM_VERSION_PATCH 0 -#define NVM_BLK_BITS (16) -#define NVM_PG_BITS (16) -#define NVM_SEC_BITS (8) -#define NVM_PL_BITS (8) -#define NVM_LUN_BITS (8) -#define NVM_CH_BITS (8) - -struct ppa_addr { - /* Generic structure for all addresses */ - union { - struct { - u64 blk : NVM_BLK_BITS; - u64 pg : NVM_PG_BITS; - u64 sec : NVM_SEC_BITS; - u64 pl : NVM_PL_BITS; - u64 lun : NVM_LUN_BITS; - u64 ch : NVM_CH_BITS; - } g; - - u64 ppa; - }; -}; - struct nvm_rq { struct nvm_tgt_instance *ins; struct nvm_dev *dev; @@ -182,39 +222,6 @@ static inline void *nvm_rq_to_pdu(struct nvm_rq *rqdata) struct nvm_block; -typedef int (nvm_l2p_update_fn)(u64, u32, __le64 *, void *); -typedef int (nvm_bb_update_fn)(struct ppa_addr, int, u8 *, void *); -typedef int (nvm_id_fn)(struct nvm_dev *, struct nvm_id *); -typedef int (nvm_get_l2p_tbl_fn)(struct nvm_dev *, u64, u32, - nvm_l2p_update_fn *, void *); -typedef int (nvm_op_bb_tbl_fn)(struct nvm_dev *, struct ppa_addr, int, - nvm_bb_update_fn *, void *); -typedef int (nvm_op_set_bb_fn)(struct nvm_dev *, struct nvm_rq *, int); -typedef int (nvm_submit_io_fn)(struct nvm_dev *, struct nvm_rq *); -typedef int (nvm_erase_blk_fn)(struct nvm_dev *, struct nvm_rq *); -typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *)
Re: Add "lightnvm: ensure that nvm_dev_ops can be used without CONFIG_NVM" to v4.4.y stable tree?
On Fri, 2016-10-14 at 15:47 +0200, Greg KH wrote: > Makes sense to me, can you forward on a patch that applies cleanly and > I'll queue it up for the next round (after this one.) Hope to do that later today. If it turns out I've messed things up Jens can still speak up. Thanks, Paul Bolle
Re: Add "x86/build: Build compressed x86 kernels as PIE" to the v4.4.y stable tree?
On Fri, 2016-10-14 at 16:08 +0200, Greg KH wrote: > Did you apply that patch and it worked properly for you? If so, yes, > I'll be glad to queue it up. Yes I did and it did. But I do find this commit as magical as they can get so I'd hoped that hjl would speak up. But if my word is enough for you, then feel free to queue it up. > Also, stable questions/issues should be cc:ed to sta...@vger.kernel.org. Sorry about that. I've received one too many of your formletters, so now I try to avoid sta...@vger.kernel.org as much as I can. Thanks, Paul Bolle