Re: [tip: sched/core] sched: Fix balance_callback()

2020-11-11 Thread Paul Bolle
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(>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

2020-11-05 Thread Paul Bolle
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

2020-10-07 Thread tip-bot2 for Paul Bolle
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

2020-10-07 Thread Paul Bolle
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

2020-10-07 Thread Paul Bolle
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

2020-10-07 Thread Paul Bolle
[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

2020-10-01 Thread Paul Bolle
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

2019-09-27 Thread Paul Bolle
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

2019-08-26 Thread Paul Bolle
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

2019-08-25 Thread Paul Bolle
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

2019-07-27 Thread Paul Bolle
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 = >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

2019-07-26 Thread Paul Bolle
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

2019-07-26 Thread Paul Bolle
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

2019-07-26 Thread Paul Bolle
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), ) == 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

2019-07-26 Thread Paul Bolle
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 = >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 = >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

2019-07-24 Thread Paul Bolle
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?

2019-04-09 Thread Paul Bolle
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?

2019-04-09 Thread Paul Bolle
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++;
-   _include();
-   _asm_types();
-   _sizetypes();
-   _declarations();
-   # Dropped for now. Too much noise _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

2019-02-12 Thread Paul Bolle
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

2019-02-09 Thread Paul Bolle
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

2018-05-18 Thread Paul Bolle
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 = _proc_fops;
> + iif->ctr.proc_show = gigaset_proc_show,
>   INIT_LIST_HEAD(>appls);
>   skb_queue_head_init(>sendqueue);
>   atomic_set(>sendqlen, 0);

Thanks,


Paul Bolle


Re: [PATCH 38/42] isdn: replace ->proc_fops with ->proc_show

2018-05-18 Thread Paul Bolle
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 = _proc_fops;
> + iif->ctr.proc_show = gigaset_proc_show,
>   INIT_LIST_HEAD(>appls);
>   skb_queue_head_init(>sendqueue);
>   atomic_set(>sendqlen, 0);

Thanks,


Paul Bolle


Re: [PATCH] kconfig: Warn if help text is blank

2018-01-30 Thread Paul Bolle
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

2018-01-30 Thread Paul Bolle
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

2018-01-30 Thread Paul Bolle
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] kconfig: Warn if help text is blank

2018-01-30 Thread Paul Bolle
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

2018-01-24 Thread Paul Bolle
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 <mawil...@microsoft.com>

> --- 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 v6 05/99] xarray: Add definition of struct xarray

2018-01-24 Thread Paul Bolle
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()

2017-10-19 Thread Paul Bolle
[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()

2017-10-19 Thread Paul Bolle
[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()

2017-10-19 Thread Paul Bolle
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(>lock);
> +  ucs->cs = cs;
> -   setup_timer(>timer_ctrl, req_timeout, (unsigned long) cs);
> -   setup_timer(>timer_atrdy, atrdy_timeout, (unsigned long) cs);
> -   setup_timer(>timer_cmd_in, cmd_in_timeout, (unsigned long) cs);
> -   setup_timer(>timer_int_in, int_in_resubmit, (unsigned long) cs);
> +   timer_setup(>timer_ctrl, req_timeout, 0);
> +   timer_setup(>timer_atrdy, atrdy_timeout, 0);
> +   timer_setup(>timer_cmd_in, cmd_in_timeout, 0);
> +   timer_setup(>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()

2017-10-19 Thread Paul Bolle
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(>lock);
> +  ucs->cs = cs;
> -   setup_timer(>timer_ctrl, req_timeout, (unsigned long) cs);
> -   setup_timer(>timer_atrdy, atrdy_timeout, (unsigned long) cs);
> -   setup_timer(>timer_cmd_in, cmd_in_timeout, (unsigned long) cs);
> -   setup_timer(>timer_int_in, int_in_resubmit, (unsigned long) cs);
> +   timer_setup(>timer_ctrl, req_timeout, 0);
> +   timer_setup(>timer_atrdy, atrdy_timeout, 0);
> +   timer_setup(>timer_cmd_in, cmd_in_timeout, 0);
> +   timer_setup(>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()

2017-10-19 Thread Paul Bolle
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(>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()

2017-10-19 Thread Paul Bolle
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(>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()

2017-10-19 Thread Paul Bolle
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 <pebo...@tiscali.nl>

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()

2017-10-19 Thread Paul Bolle
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()

2017-10-19 Thread Paul Bolle
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 <pebo...@tiscali.nl>

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 32/58] isdn/gigaset: Convert timers to use timer_setup()

2017-10-19 Thread Paul Bolle
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

2017-10-19 Thread Paul Bolle
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 <pebo...@tiscali.nl>

Thanks,


Paul Bolle


Re: [PATCH 31/58] isdn/gigaset: Use kzalloc instead of open-coded field zeroing

2017-10-19 Thread Paul Bolle
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()

2017-10-06 Thread Paul Bolle
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 v2] isdn/gigaset: Convert timers to use timer_setup()

2017-10-06 Thread Paul Bolle
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()

2017-10-05 Thread Paul Bolle
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] isdn/gigaset: Convert timers to use timer_setup()

2017-10-05 Thread Paul Bolle
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()

2017-06-09 Thread Paul Bolle
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 v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others()

2017-06-09 Thread Paul Bolle
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

2017-02-17 Thread Paul Bolle
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 <j...@perches.com>
> ---
>  drivers/isdn/gigaset/interface.c|  2 +-

Gigaset hunk looks good to me:
    Acked-by: Paul Bolle <pebo...@tiscali.nl>

>  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 20/35] drivers/isdn: Convert remaining uses of pr_warning to pr_warn

2017-02-17 Thread Paul Bolle
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

2017-02-03 Thread Paul Bolle
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 <nicolas.dich...@6wind.com>

I've got an identical patch in my ever growing stack of stuff that I should
actually submit:

Acked-by: Paul Bolle <pebo...@tiscali.nl>

Thanks,


Paul Bolle


Re: [PATCH v5 5/8] Makefile.headersinst: remove destination-y option

2017-02-03 Thread Paul Bolle
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

2017-01-03 Thread Paul Bolle
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

2017-01-03 Thread Paul Bolle
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

2017-01-03 Thread Paul Bolle
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] isdn: i4l: move active-isdn drivers to staging

2017-01-03 Thread Paul Bolle
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

2016-12-21 Thread Paul Bolle
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

2016-12-21 Thread Paul Bolle
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

2016-12-21 Thread Paul Bolle
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


Re: [PATCH 2/2] net: wireless: fix to uses struct

2016-12-21 Thread Paul Bolle
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/

2016-12-20 Thread Paul Bolle
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 <pebo...@tiscali.nl>
---
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



[PATCH] checkpatch: stop worrying about include/asm/

2016-12-20 Thread Paul Bolle
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

2016-12-01 Thread Paul Bolle
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

2016-12-01 Thread Paul Bolle
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

2016-12-01 Thread Paul Bolle
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

2016-12-01 Thread Paul Bolle
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

2016-12-01 Thread Paul Bolle
[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

2016-12-01 Thread Paul Bolle
[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

2016-11-30 Thread Paul Bolle
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

2016-11-30 Thread Paul Bolle
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

2016-11-30 Thread Paul Bolle
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

2016-11-30 Thread Paul Bolle
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

2016-11-30 Thread Paul Bolle
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: Odd build breakage in 4.9-rc7

2016-11-30 Thread Paul Bolle
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

2016-11-28 Thread Paul Bolle
[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


Re: [PATCH] Scripts: kconfig: nconf: fix _GNU_SOURCE redefined warning

2016-11-28 Thread Paul Bolle
[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

2016-11-27 Thread tip-bot for Paul Bolle
Commit-ID:  9190e21780dfeff524a67c6e7b806c8a9d496086
Gitweb: http://git.kernel.org/tip/9190e21780dfeff524a67c6e7b806c8a9d496086
Author: Paul Bolle <pebo...@tiscali.nl>
AuthorDate: Fri, 25 Nov 2016 13:41:47 +0100
Committer:  Ingo Molnar <mi...@kernel.org>
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 <pebo...@tiscali.nl>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Link: 
http://lkml.kernel.org/r/1480077707-2837-1-git-send-email-pebo...@tiscali.nl
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
 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: Remove three unneeded genhdr-y entries

2016-11-27 Thread tip-bot for Paul Bolle
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

2016-11-27 Thread tip-bot for Paul Bolle
Commit-ID:  06cbbac0f57d947656a12c30a0a69d4cf0ac6dea
Gitweb: http://git.kernel.org/tip/06cbbac0f57d947656a12c30a0a69d4cf0ac6dea
Author: Paul Bolle <pebo...@tiscali.nl>
AuthorDate: Fri, 25 Nov 2016 13:38:34 +0100
Committer:  Ingo Molnar <mi...@kernel.org>
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 <pebo...@tiscali.nl>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Masahiro Yamada <yamada.masah...@socionext.com>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Link: 
http://lkml.kernel.org/r/1480077514-2586-1-git-send-email-pebo...@tiscali.nl
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
 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


[tip:x86/build] x86/build: Don't use $(LINUXINCLUDE) twice

2016-11-27 Thread tip-bot for Paul Bolle
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

2016-11-25 Thread Paul Bolle
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 <pebo...@tiscali.nl>
---
 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/build: remove three unneeded genhdr-y entries

2016-11-25 Thread Paul Bolle
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

2016-11-25 Thread Paul Bolle
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 <pebo...@tiscali.nl>
---
 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] x86/realmode: Don't use $(LINUXINCLUDE) twice

2016-11-25 Thread Paul Bolle
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

2016-11-25 Thread Paul Bolle
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 <pebo...@tiscali.nl>
---
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] MIPS: ZBOOT: Don't use $(LINUXINCLUDE) twice

2016-11-25 Thread Paul Bolle
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)

2016-11-25 Thread Paul Bolle
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 <pebo...@tiscali.nl>
---
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



[PATCH] cris: No need to append -O2 and $(LINUXINCLUDE)

2016-11-25 Thread Paul Bolle
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

2016-11-24 Thread Paul Bolle
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=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: Linux 4.9: Reported regressions as of Sunday, 2016-11-20

2016-11-24 Thread Paul Bolle
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=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

2016-11-17 Thread Paul Bolle
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

2016-11-17 Thread Paul Bolle
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

2016-11-17 Thread Paul Bolle
[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 <pebo...@tiscali.nl>
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 <pebo...@tiscali.nl>
Acked-by: Michal Marek <mma...@suse.cz> (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: i2c: undefined option I2C_ALGO_BUSCLEAR

2016-11-17 Thread Paul Bolle
[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

2016-11-14 Thread Paul Bolle
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] [media] gp8psk-fe: add missing MODULE_foo() macros

2016-11-14 Thread Paul Bolle
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

2016-11-08 Thread Paul Bolle
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 2/2] s390: squash facilities_src.h into gen_facilities.c

2016-11-08 Thread Paul Bolle
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

2016-11-08 Thread Paul Bolle
Hi Mashiro,

On Tue, 2016-11-08 at 10:50 +0900, Masahiro Yamada wrote:
> 2016-11-07 21:52 GMT+09:00 Paul Bolle <pebo...@tiscali.nl>:
> > 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 1/2] s390: delete unneeded #include from facilities_src.h

2016-11-08 Thread Paul Bolle
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

2016-11-07 Thread Paul Bolle
On Mon, 2016-11-07 at 10:50 +0100, Martin Schwidefsky wrote:
> Heiko Carstens <heiko.carst...@de.ibm.com> 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 <yamada.masah...@socionext.com>

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 e

Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

2016-11-07 Thread Paul Bolle
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-exception-fetch/

  1   2   3   4   5   6   7   8   9   10   >