Re: [net-next] intel: add SPDX identifiers to all the Intel drivers
Allan, On Tue, Mar 20, 2018 at 1:48 PM, Allan, Bruce W <bruce.w.al...@intel.com> wrote: >> -Original Message- >> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] >> On Behalf Of Jeff Kirsher >> Sent: Tuesday, March 20, 2018 10:52 AM >> To: Joe Perches <j...@perches.com>; da...@davemloft.net; Philippe >> Ombredanne <pombreda...@nexb.com> >> Cc: netdev@vger.kernel.org; nhor...@redhat.com; sassm...@redhat.com; >> jogre...@redhat.com >> Subject: Re: [net-next] intel: add SPDX identifiers to all the Intel drivers >> >> On Tue, 2018-03-20 at 10:41 -0700, Joe Perches wrote: >> > On Tue, 2018-03-20 at 10:13 -0700, Jeff Kirsher wrote: >> > > Add the SPDX identifiers to all the Intel wired LAN driver files, >> > > as >> > > outlined in Documentation/process/license-rules.rst. >> > >> > So far the Documentation does not show using the -only variant. >> > >> > For a discussion, please see: >> > https://lkml.org/lkml/2018/2/8/311 > > But the Linux Foundation, the authority maintaining the valid SPDX > identifiers, indicates at https://spdx.org/licenses/ that "GPL-2.0" is > deprecated while "GPL-2.0-only" (and others) is appropriate. > Was there any mention in the thread or other conversations if/when the > kernel's documentation (and all existing uses of "GPL-2.0" in the kernel) > will be updated to "GPL-2.0-only"? The kernel (as documented by Thomas [1]) is using for now the V2.6 of the SPDX licenses list. [2] IMHO the reference should be the kernel doc and nothing else to ensure consistency and avoid confusion (which obviously was not avoided entirely here ;) ). What happened is in late December a new version 3 was published by SPDX and the v2.6 is no longer online. I will bring this up to the SPDX group because we should be able to reference the version 2.6 online (it is still in git though [2]). When the kernel maintainers decide to switch to V3.0 of the SPDX list, the doc will be updated and then Joe's script could be applied at once to update the past. What matters most here is consistency: having some v2.6 and some v3.0 SPDX ids at once is not a happy thing IMHO. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst [2] https://github.com/spdx/license-list-data/tree/v2.6 >> :-( I had it originally as GPL-2.0 and then it was pointed out that it >> was being deprecated, so rather than creating future thrash over the >> change, figured I would be ahead of the game. >> >> > >> > > diff --git a/drivers/net/ethernet/intel/e100.c >> > > b/drivers/net/ethernet/intel/e100.c >> > >> > [] >> > > @@ -1,3 +1,4 @@ >> > > +// SPDX-License-Identifier: GPL-2.0-only >> > >> > etc... -- Cordially Philippe Ombredanne
Re: [PATCH v3 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Hi Moritz,
On Fri, Feb 16, 2018 at 6:00 PM, Moritz Fischer <m...@kernel.org> wrote:
> Add support for the National Instruments XGE 1/10G network device.
>
> It uses the EEPROM on the board via NVMEM.
>
> Signed-off-by: Moritz Fischer <m...@kernel.org>
> --- /dev/null
> +++ b/drivers/net/ethernet/ni/nixge.c
> @@ -0,0 +1,1352 @@
> +// SPDX-License-Identifier: GPL-2.0
> +MODULE_LICENSE("GPL");
This does not match your license above. Per module.h "GPL" would mean "GPL-2.0+"
Can you use one or the other an\d ensure both of these are in sync?
--
Cordially
Philippe Ombredanne
Re: [PATCH 6/6] add test for aio poll and io_pgetevents
Dear Christoph, On Thu, Jan 4, 2018 at 9:03 AM, Christoph Hellwig <h...@lst.de> wrote: > Signed-off-by: Christoph Hellwig <h...@lst.de> > --- > harness/cases/22.t | 149 > + > 1 file changed, 149 insertions(+) > create mode 100644 harness/cases/22.t > > diff --git a/harness/cases/22.t b/harness/cases/22.t > new file mode 100644 > index 000..a9fec6b > --- /dev/null > +++ b/harness/cases/22.t > @@ -0,0 +1,149 @@ > +/* > + * Copyright (C) 2006-2018 Free Software Foundation, Inc. > + * Copyright (C) 2018 Christoph Hellwig. > + * License: LGPLv2.1 or later. Would you consider using an SPDX tag instead as documented in Thomas doc patches [1]? This rather close to what you use today and would come out as this, on the first line: SPDX-License-Identifier: LGPL-2.1+ Thank you! [1] https://lkml.org/lkml/2017/12/28/323 -- Cordially Philippe Ombredanne, your kernel licensing scruffy
Re: [PATCH bpf-next v5 3/4] libbpf: add missing SPDX-License-Identifier
On Thu, Jan 4, 2018 at 9:21 AM, Eric Leblond <e...@regit.org> wrote: > Signed-off-by: Eric Leblond <e...@regit.org> > Acked-by: Alexei Starovoitov <a...@kernel.org> > --- > tools/lib/bpf/bpf.c| 2 ++ > tools/lib/bpf/bpf.h| 2 ++ > tools/lib/bpf/libbpf.c | 2 ++ > tools/lib/bpf/libbpf.h | 2 ++ > 4 files changed, 8 insertions(+) > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > index 10d71b9fdbd0..38d720466fe8 100644 > --- a/tools/lib/bpf/bpf.c > +++ b/tools/lib/bpf/bpf.c > @@ -1,3 +1,5 @@ > +// SPDX-License-Identifier: LGPL-2.1 > + > /* > * common eBPF ELF operations. > * > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h > index 9f44c196931e..8d18fb73d7fb 100644 > --- a/tools/lib/bpf/bpf.h > +++ b/tools/lib/bpf/bpf.h > @@ -1,3 +1,5 @@ > +/* SPDX-License-Identifier: LGPL-2.1 */ > + > /* > * common eBPF ELF operations. > * > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 5fe8aaa2123e..924a8b8431ab 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -1,3 +1,5 @@ > +// SPDX-License-Identifier: LGPL-2.1 > + > /* > * Common eBPF ELF object loading operations. > * > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index e42f96900318..f85906533cdd 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -1,3 +1,5 @@ > +/* SPDX-License-Identifier: LGPL-2.1 */ > + > /* > * Common eBPF ELF object loading operations. > * > -- > 2.15.1 > Reviewed-by: Philippe Ombredanne <pombreda...@nexb.com> Thank you for using the SPDX tags! -- Cordially Philippe Ombredanne
Re: [PATCH bpf-next v4 2/3] libbpf: add error reporting in XDP
On Sat, Dec 30, 2017 at 9:41 PM, Eric Leblond <e...@regit.org> wrote: > Parse netlink ext attribute to get the error message returned by > the card. Code is partially take from libnl. > > Signed-off-by: Eric Leblond <e...@regit.org> > Acked-by: Alexei Starovoitov <a...@kernel.org> > --- /dev/null > +++ b/tools/lib/bpf/nlattr.c > @@ -0,0 +1,187 @@ > +// SPDX-License-Identifier: LGPL-2.1 > + > +/* > + * NETLINK Netlink attributes > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation version 2.1 > + * of the License. > + * > + * Copyright (c) 2003-2013 Thomas Graf <tg...@suug.ch> > + */ Do you think you could get an ack or signoff from the author (i.e.Thomas Graf) to get a more streamlined thing such as this: > +// SPDX-License-Identifier: LGPL-2.1 > +// NETLINK Netlink attributes > +// Copyright (c) 2003-2013 Thomas Graf <tg...@suug.ch> -- Cordially Philippe Ombredanne
Re: [PATCH bpf-next v3 3/3] libbpf: add missing SPDX-License-Identifier
Eric, On Thu, Dec 28, 2017 at 9:04 AM, Eric Leblond <e...@regit.org> wrote: > Signed-off-by: Eric Leblond <e...@regit.org> > Acked-by: Alexei Starovoitov <a...@kernel.org> > --- > tools/lib/bpf/bpf.c| 2 ++ > tools/lib/bpf/bpf.h| 2 ++ > tools/lib/bpf/libbpf.c | 2 ++ > tools/lib/bpf/libbpf.h | 2 ++ > 4 files changed, 8 insertions(+) > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > index cdfabbe118cc..9e53dbbca2bd 100644 > --- a/tools/lib/bpf/bpf.c > +++ b/tools/lib/bpf/bpf.c > @@ -1,3 +1,5 @@ > +/* SPDX-License-Identifier: LGPL-2.1 */ In a .c file these should using C++-style comments as in //. As requested by Linus, discussed on list and documented in Thomas doc patches. > + > /* > * common eBPF ELF operations. > * > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h > index 9f44c196931e..8d18fb73d7fb 100644 > --- a/tools/lib/bpf/bpf.h > +++ b/tools/lib/bpf/bpf.h > @@ -1,3 +1,5 @@ > +/* SPDX-License-Identifier: LGPL-2.1 */ And this is correct for a .h > + > /* > * common eBPF ELF operations. > * > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 5fe8aaa2123e..878e240a681b 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -1,3 +1,5 @@ > +/* SPDX-License-Identifier: LGPL-2.1 */ Use // here. > + > /* > * Common eBPF ELF object loading operations. > * > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index e42f96900318..f85906533cdd 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -1,3 +1,5 @@ > +/* SPDX-License-Identifier: LGPL-2.1 */ > + > /* > * Common eBPF ELF object loading operations. > * > -- > 2.15.1 > -- Cordially Philippe Ombredanne
Re: [PATCH v4 net-next 0/8] Hisilicon Network Subsystem 3 VF Ethernet Driver
Salil, On Thu, Dec 14, 2017 at 7:03 PM, Salil Mehta <salil.me...@huawei.com> wrote: > This patch-set contains the support of the HNS3 (Hisilicon Network Subsystem > 3) > Virtual Function Ethernet driver for hip08 family of SoCs. The Physical > Function > driver is already part of the Linux mainline. > > This VF driver has its Hardware Compatibility Layer and has commom/unified > ENET > layer/client/ethtool code with the PF driver. It also has support of mailbox > to > communicate with the HNS3 PF driver. The basic architecture of VF driver is > derivative of the PF driver. Just like PF driver, this driver is also PCI > Express based. > > Change Log Summary: > Patch V4: Addressed SPDX related comment by Philippe Ombredanne > Patch V3: Addressed SPDX change requested by Philippe Ombredanne Thank you. For the use of SPDX tags (and this only!) you have my cheerful ack for the whole patch set. Acked-by: Philippe Ombredanne <pombreda...@nexb.com> -- Cordially Philippe Ombredanne
Re: [PATCH V3 net-next 3/8] net: hns3: Add HNS3 VF HCL(Hardware Compatibility Layer) Support
Dear Salil, On Wed, Dec 13, 2017 at 11:35 AM, Salil Mehta <salil.me...@huawei.com> wrote: > I could also make out from different articles, including from the below, > Linus suggesting moving to "//" type instead of starred ones for headers. > > It looks SPDX change is still a suggestion? Well, I think this has been discussed and agreed among maintainers at the plumbers summit and here on this list. And when I see Thomas doc patches, Greg patches, other key maintainers as well as Linus comments and reviews and guidelines, I can can hardly see this as just being a "suggestion", but that's your call to consider it this way. -- Cordially Philippe Ombredanne
Re: [PATCH V3 net-next 3/8] net: hns3: Add HNS3 VF HCL(Hardware Compatibility Layer) Support
Dear Salil,
On Tue, Dec 12, 2017 at 6:52 PM, Salil Mehta <salil.me...@huawei.com> wrote:
> This patch adds the support of hardware compatibiltiy layer to the
> HNS3 VF Driver. This layer implements various {set|get} operations
> over MAC address for a virtual port, RSS related configuration,
> fetches the link status info from PF, does various VLAN related
> configuration over the virtual port, queries the statistics from
> the hardware etc.
>
> This layer can directly interact with hardware through the
> IMP(Integrated Mangement Processor) interface or can use mailbox
> to interact with the PF driver.
>
> Signed-off-by: Salil Mehta <salil.me...@huawei.com>
> Signed-off-by: lipeng <lipeng...@huawei.com>
> ---
> Patch V3: Addressed SPDX change requested by Philippe Ombredanne
> Link: https://lkml.org/lkml/2017/12/8/874
> Patch V2: Addressed some internal comments
> Patch V1: Initial Submit
> ---
> .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 1490
>
> .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 164 +++
> 2 files changed, 1654 insertions(+)
> create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> new file mode 100644
> index 000..ff55f4c
> --- /dev/null
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> @@ -0,0 +1,1490 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2016-2017 Hisilicon Limited.
> + */
This is just me nitpicking and this is entirely up to you but in
such a simple case you could go all the way too:
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (c) 2016-2017 Hisilicon Limited.
In this case this can make the thing look more consistent.
See also Linus commentaries about this [1][2][3][4]
[1] https://lkml.org/lkml/2017/11/25/133
[2] https://lkml.org/lkml/2017/11/25/125
[3] https://lkml.org/lkml/2017/11/2/715
[4] https://lkml.org/lkml/2017/11/2/805
--
Cordially
Philippe Ombredanne
Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver
Dear Masami-san, On Tue, Dec 12, 2017 at 3:29 AM, Masami Hiramatsu <masami.hirama...@linaro.org> wrote: [...] > Then what I'm considering is copyright notice lines. Those are usually > treat as the header lines, not single line. So > >> +// SDPX-License-Identifier: GPL-2.0 >> +// sni_ave.c - Socionext UniPhier AVE ethernet driver >> +// Copyright 2014 Panasonic Corporation >> +// Copyright 2015-2017 Socionext Inc. > > is acceptable? or should we keep C-style header lines for new drivers? > >> +// SDPX-License-Identifier: GPL-2.0 >> +/* >> + * sni_ave.c - Socionext UniPhier AVE ethernet driver >> + * Copyright 2014 Panasonic Corporation >> + * Copyright 2015-2017 Socionext Inc. >> + */ > > I just concern that those lines are not "single". that's all. :) My voice carries the weight of a down feather in this discussion and to me the benefit of the first form is that you have removed two lines. Both forms work fine. -- Cordially Philippe Ombredanne
Re: [PATCH net-next v5 0/2] net: thunderx: add support for PTP clock
Aleksey, On Mon, Dec 11, 2017 at 3:14 PM, Aleksey Makarov <aleksey.maka...@cavium.com> wrote: > This series adds support for IEEE 1588 Precision Time Protocol > to Cavium ethernet driver. > > The first patch adds support for the Precision Time Protocol Clocks and > Timestamping coprocessor (PTP) found on Cavium processors. > It registers a new PTP clock in the PTP core and provides functions > to use the counter in BGX, TNS, GTI, and NIC blocks. > > The second patch introduces support for the PTP protocol to the > Cavium ThunderX ethernet driver. > > v5: > - fix the file headers (add SPDX tags, remove advertisment) (Philippe > Ombredanne) Thank you. Acked-by: Philippe Ombredanne <pombreda...@nexb.com> -- Cordially Philippe Ombredanne
Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver
Dear Kunihiko-san, On Mon, Dec 11, 2017 at 8:57 AM, Kunihiko Hayashi <hayashi.kunih...@socionext.com> wrote: > The UniPhier platform from Socionext provides the AVE ethernet > controller that includes MAC and MDIO bus supporting RGMII/RMII > modes. The controller is named AVE. > > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org> > Reviewed-by: Andrew Lunn <and...@lunn.ch> [...] > --- /dev/null > +++ b/drivers/net/ethernet/socionext/Makefile > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0 You are correctly using SPDX ids here > +obj-$(CONFIG_SNI_AVE) += sni_ave.o > diff --git a/drivers/net/ethernet/socionext/sni_ave.c > b/drivers/net/ethernet/socionext/sni_ave.c > new file mode 100644 > index 000..7b293c2 > --- /dev/null > +++ b/drivers/net/ethernet/socionext/sni_ave.c > @@ -0,0 +1,1744 @@ > +/** > + * sni_ave.c - Socionext UniPhier AVE ethernet driver > + * > + * Copyright 2014 Panasonic Corporation > + * Copyright 2015-2017 Socionext Inc. > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ ... then I guess you could also use them here, replacing at least 7 lines of boilerplate by a single id line? > +// SDPX-License-Identifier: GPL-2.0 And if you go C++ style all the way, this could be even more compact: > +// SDPX-License-Identifier: GPL-2.0 > +// sni_ave.c - Socionext UniPhier AVE ethernet driver > +// Copyright 2014 Panasonic Corporation > +// Copyright 2015-2017 Socionext Inc. Thank you for your kind consideration! -- Cordially Philippe Ombredanne
Re: [PATCH V2 net-next 2/8] net: hns3: Add mailbox support to VF driver
Sali,
On Fri, Dec 8, 2017 at 10:16 PM, Salil Mehta <salil.me...@huawei.com> wrote:
> This patch adds the support of the mailbox to the VF driver. The
> mailbox shall be used as an interface to communicate with the
> PF driver for various purposes like {set|get} MAC related
> operations, reset, link status etc. The mailbox supports both
> synchronous and asynchronous command send to PF driver.
>
> Signed-off-by: Salil Mehta <salil.me...@huawei.com>
> Signed-off-by: lipeng <lipeng...@huawei.com>
[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (c) 2016-2017 Hisilicon Limited.
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> + */
Why not use the new SPDX ids?
e.g.
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (c) 2016-2017 Hisilicon Limited. */
See Linus posts and Thomas doc patches for details.
--
Cordially
Philippe Ombredanne
Re: [PATCH v3] leds: trigger: Introduce a NETDEV trigger
Pavel, On Fri, Dec 8, 2017 at 3:27 PM, Pavel Machek <pa...@ucw.cz> wrote: > On Thu 2017-12-07 14:01:39, Philippe Ombredanne wrote: >> Ben, >> >> On Thu, Dec 7, 2017 at 12:46 PM, Ben Whitten <ben.whit...@gmail.com> wrote: >> > From: Ben Whitten <ben.whit...@gmail.com> >> > >> > This commit introduces a NETDEV trigger for named device >> > activity. Available triggers are link, rx, and tx. >> > >> > Signed-off-by: Ben Whitten <ben.whit...@gmail.com> >> [] >> > --- /dev/null >> > +++ b/drivers/leds/trigger/ledtrig-netdev.c >> > @@ -0,0 +1,503 @@ >> > +/* >> > + * LED Kernel Netdev Trigger >> > + * >> > + * Toggles the LED to reflect the link and traffic state of a named net >> > device >> > + * >> > + * Copyright 2017 Ben Whitten <ben.whit...@gmail.com> >> > + * >> > + * Copyright 2007 Oliver Jowett <oli...@opencloud.com> >> > + * >> > + * Derived from ledtrig-timer.c which is: >> > + * Copyright 2005-2006 Openedhand Ltd. >> > + * Author: Richard Purdie <rpur...@openedhand.com> >> > + * >> > + * This program is free software; you can redistribute it and/or modify >> > + * it under the terms of the GNU General Public License version 2 as >> > + * published by the Free Software Foundation. >> > + * >> > + */ >> >> Have you considered using the new SPDX id instead ? See Thomas doc >> patches and Greg and Linus comments on the topic >> Here this would likely come out this way (yes, using a C++ comment at the >> top): >> >> > +// SPDX-License-Identifier: GPL-2.0 >> > +/* >> > + * LED Kernel Netdev Trigger >> > + * >> > + * Toggles the LED to reflect the link and traffic state of a named net >> > device >> > + * >> > + * Copyright 2017 Ben Whitten <ben.whit...@gmail.com> >> > + * >> > + * Copyright 2007 Oliver Jowett <oli...@opencloud.com> >> > + * >> > + * Derived from ledtrig-timer.c which is: >> > + * Copyright 2005-2006 Openedhand Ltd. >> > + * Author: Richard Purdie <rpur...@openedhand.com> >> > + * >> > + */ >> >> >> This is cleaner and simpler, don't you think? > > Please consider putting SPDX where it logically belongs -- near the > copyright. Linus wants it on the first line. So you could write this way alright: >> > +// SPDX-License-Identifier: GPL-2.0 >> > +// Copyright 2017 Ben Whitten <ben.whit...@gmail.com> >> > +// Copyright 2007 Oliver Jowett <oli...@opencloud.com> >> > +/* >> > + * LED Kernel Netdev Trigger >> > + * >> > + * Toggles the LED to reflect the link and traffic state of a named net >> > device >> > + * >> > + * Derived from ledtrig-timer.c which is: >> > + * Copyright 2005-2006 Openedhand Ltd. >> > + * Author: Richard Purdie <rpur...@openedhand.com> >> > + * >> > + */ For reference please check Linus [1][2][3], Thomas [4] and Greg [5] comments on the topic of C++ style // comments and first line placement. Jonathan also wrote a nice background article on the SPDXification topic at LWN [6] [1] https://lkml.org/lkml/2017/11/2/715 [2] https://lkml.org/lkml/2017/11/25/125 [3] https://lkml.org/lkml/2017/11/25/133 [4] https://lkml.org/lkml/2017/11/2/805 [5] https://lkml.org/lkml/2017/10/19/165 [6] https://lwn.net/Articles/739183/ -- Cordially Philippe Ombredanne
Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
On Fri, Dec 8, 2017 at 11:34 AM, Quentin Monnet <quentin.mon...@netronome.com> wrote: > 2017-12-07 18:39 UTC+ ~ Roman Gushchin <g...@fb.com> >> This patch adds basic cgroup bpf operations to bpftool: >> cgroup list, attach and detach commands. [...] >> --- /dev/null >> +++ b/tools/bpf/bpftool/cgroup.c >> @@ -0,0 +1,305 @@ >> +/* >> + * Copyright (C) 2017 Facebook >> + * >> + * 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; either version >> + * 2 of the License, or (at your option) any later version. >> + * >> + * >> + */ >> + Roman, Have you considered using the simpler and new SPDX ids instead? e.g.: // SPDX-License-Identifier: GPL-2.0+ // Copyright (C) 2017 Facebook // Author: Roman Gushchin <g...@fb.com> This would boost your code/comments ratio nicely IMHO. For reference please check Linus [1][2][3], Thomas [4] and Greg [5] comments on the topic of C++ style // comments! Jonathan also wrote a nice background article on the SPDXification topic at LWN [6] PS: and if you could spread the word at FB, that would we awesome! [1] https://lkml.org/lkml/2017/11/2/715 [2] https://lkml.org/lkml/2017/11/25/125 [3] https://lkml.org/lkml/2017/11/25/133 [4] https://lkml.org/lkml/2017/11/2/805 [5] https://lkml.org/lkml/2017/10/19/165 [6] https://lwn.net/Articles/739183/ -- Cordially Philippe Ombredanne
Re: [PATCH net-next v4 1/2] net: add support for Cavium PTP coprocessor
Dear Aleksey, Dear Radoslaw, On Fri, Dec 8, 2017 at 11:34 AM, Aleksey Makarov <aleksey.maka...@cavium.com> wrote: > From: Radoslaw Biernacki <r...@semihalf.com> > > This patch adds support for the Precision Time Protocol > Clocks and Timestamping hardware found on Cavium ThunderX > processors. > > Signed-off-by: Radoslaw Biernacki <r...@semihalf.com> > Signed-off-by: Aleksey Makarov <aleksey.maka...@cavium.com> [] > --- /dev/null > +++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c > @@ -0,0 +1,334 @@ > +/* > + * cavium_ptp.c - PTP 1588 clock on Cavium hardware > + * > + * Copyright (c) 2003-2015, 2017 Cavium, Inc. > + * > + * 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; either version 2 of the License. Have you considered using the new SPDX ids instead of this fine legalese? e.g.: // SPDX-License-Identifier: GPL-2.0 This is much shorter and neater (unless you are a legalese lover of course!)... Check also Thomas doc patches and Linus comments on why he prefers the C++ comment style for these. And even better, what about this more compact form? I am a big fan of the C++ style comments for these (and so is Linus): // SPDX-License-Identifier: GPL-2.0 // Copyright (c) 2003-2015, 2017 Cavium, Inc. // cavium_ptp.c - PTP 1588 clock on Cavium hardware > + * > + * This file may also be available under a different license from Cavium. > + * Contact Cavium, Inc. for more information > + */ I am not so sure that the kernel source tree is the right place for commercial advertisement... I mean, this is a fine statement to put on your company web site, but who reads this code: you and I do we care seriously about this? Anyone who uses your hardware would likely have some other kind of arrangements with your company anyway, making this sentence moot in all cases. Therefore I tend to think this is just a noisy distraction from your otherwise fine code contributions: It does not come out to me as kernically-correct ;) So can you consider removing this fine marketing statement from this and other Cavium past, present and future contributions? That would be much appreciated! (and while you are at it, using SPDX ids throughout would give you good karma extra points) PS: Now, if this "different license" of yours is a fine BSD or MIT, you could use instead this SPDX shorthand to the same effect without turning the kernel code into a billboard: // SPDX-License-Identifier: (GPL-2.0 OR MIT) -- Cordially Philippe Ombredanne, your ad-sensitive licensing scruffy
Re: [PATCH v3 04/33] nds32: Kernel booting and initialization
Dear Greentime, On Fri, Dec 8, 2017 at 10:11 AM, Greentime Hu <green...@gmail.com> wrote: > From: Greentime Hu <greent...@andestech.com> > > This patch includes the kernel startup code. It can get dtb pointer > passed from bootloader. It will create a temp mapping by tlb > instructions at beginning and goto start_kernel. > > Signed-off-by: Vincent Chen <vince...@andestech.com> > Signed-off-by: Greentime Hu <greent...@andestech.com> [] > --- /dev/null > +++ b/arch/nds32/kernel/head.S > @@ -0,0 +1,202 @@ > +/* > + * Copyright (C) 2005-2017 Andes Technology Corporation > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ Have you considered using the new SPDX ids instead of this fine legalese? e.g.: // SPDX-License-Identifier: GPL-2.0 // Copyright (C) 2005-2017 Andes Technology Corporation This is much shorter and neater (unless you are a legalese lover of course!) Check also Thomas doc patches and Linus comments on why he prefers the C++ comment style for these. -- Cordially Philippe Ombredanne, your friendly licensing scruffy bot
Re: [PATCH v6 net-next,mips 0/7] Cavium OCTEON-III network driver.
David, On Fri, Dec 8, 2017 at 1:09 AM, David Daney <david.da...@cavium.com> wrote: [] > Changes in v5: [] > o Removed redundant licensing text boilerplate. Thank you very much! Acked-by: Philippe Ombredanne <pombreda...@nexb.com> -- Cordially Philippe Ombredanne, the licensing scruffy
Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
Roman, On Thu, Dec 7, 2017 at 11:23 PM, Jakub Kicinski <jakub.kicin...@netronome.com> wrote: > On Thu, 7 Dec 2017 18:39:09 +, Roman Gushchin wrote: >> This patch adds basic cgroup bpf operations to bpftool: >> cgroup list, attach and detach commands. >> >> Usage is described in the corresponding man pages, >> and examples are provided. >> >> Syntax: >> $ bpftool cgroup list CGROUP >> $ bpftool cgroup attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS] >> $ bpftool cgroup detach CGROUP ATTACH_TYPE PROG >> >> Signed-off-by: Roman Gushchin <g...@fb.com> > > Looks good, a few very minor nits/questions below. > >> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c >> new file mode 100644 >> index ..88d67f74313f >> --- /dev/null >> +++ b/tools/bpf/bpftool/cgroup.c >> @@ -0,0 +1,305 @@ >> +/* >> + * Copyright (C) 2017 Facebook >> + * >> + * 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; either version >> + * 2 of the License, or (at your option) any later version. >> + * >> + * Author: Roman Gushchin <g...@fb.com> >> + */ Have you considered using the new SPDX ids instead? e.g. may be something like: // SPDX-License-Identifier: GPL-2.0+ // Copyright (C) 2017 Facebook // Author: Roman Gushchin <g...@fb.com> Don't you love it with less boilerplate and a better code/comments ratio? BTW the comment style may surprise you here: this is a suggestion, but not just. Check the posts from Linus on this topic and Thomas's doc patches for the rationale. Thank you for your kind consideration! -- Cordially Philippe Ombredanne
Re: [PATCH v3] leds: trigger: Introduce a NETDEV trigger
Ben, On Thu, Dec 7, 2017 at 12:46 PM, Ben Whitten <ben.whit...@gmail.com> wrote: > From: Ben Whitten <ben.whit...@gmail.com> > > This commit introduces a NETDEV trigger for named device > activity. Available triggers are link, rx, and tx. > > Signed-off-by: Ben Whitten <ben.whit...@gmail.com> [] > --- /dev/null > +++ b/drivers/leds/trigger/ledtrig-netdev.c > @@ -0,0 +1,503 @@ > +/* > + * LED Kernel Netdev Trigger > + * > + * Toggles the LED to reflect the link and traffic state of a named net > device > + * > + * Copyright 2017 Ben Whitten <ben.whit...@gmail.com> > + * > + * Copyright 2007 Oliver Jowett <oli...@opencloud.com> > + * > + * Derived from ledtrig-timer.c which is: > + * Copyright 2005-2006 Openedhand Ltd. > + * Author: Richard Purdie <rpur...@openedhand.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ Have you considered using the new SPDX id instead ? See Thomas doc patches and Greg and Linus comments on the topic Here this would likely come out this way (yes, using a C++ comment at the top): > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * LED Kernel Netdev Trigger > + * > + * Toggles the LED to reflect the link and traffic state of a named net > device > + * > + * Copyright 2017 Ben Whitten <ben.whit...@gmail.com> > + * > + * Copyright 2007 Oliver Jowett <oli...@opencloud.com> > + * > + * Derived from ledtrig-timer.c which is: > + * Copyright 2005-2006 Openedhand Ltd. > + * Author: Richard Purdie <rpur...@openedhand.com> > + * > + */ This is cleaner and simpler, don't you think? -- Cordially Philippe Ombredanne
Re: [PATCH v5 0/6] enable creating [k,u]probe with perf_event_open
Song, On Wed, Dec 6, 2017 at 11:45 PM, Song Liu <songliubrav...@fb.com> wrote: > Changes PATCH v4 to PATCH v5: > Remove PERF_PROBE_CONFIG_IS_RETPROBE from uapi, use PMU_FORMAT_ATTR > instead. > > Changes PATCH v3 to PATCH v4: > Remove uapi define MAX_PROBE_FUNC_NAME_LEN, use KSYM_NAME_LEN instead. > Add flag PERF_PROBE_CONFIG_IS_RETPROBE for config field of [k,u]probe. > Optimize ifdef's of CONFIG_KPROBE_EVENTS and CONFIG_UPROBE_EVENTS. > Optimize checks in perf_event_is_tracing(). > Optimize perf_tp_register(). > > Changes PATCH v2 to PATCH v3: > Remove fixed type PERF_TYPE_KPROBE and PERF_TYPE_UPROBE, use dynamic > type instead. > Update userspace (samples/bpf, bcc) to look up type from sysfs. > Change License info in test_many_kprobe_user.c as Philippe Ombredanne > suggested. > > Changes PATCH v1 to PATCH v2: > Split PERF_TYPE_PROBE into PERF_TYPE_KPROBE and PERF_TYPE_UPROBE. > Split perf_probe into perf_kprobe and perf_uprobe. > Remove struct probe_desc, use config1 and config2 instead. > > Changes RFC v2 to PATCH v1: > Check type PERF_TYPE_PROBE in perf_event_set_filter(). > Rebase on to tip perf/core. > > Changes RFC v1 to RFC v2: > Fix build issue reported by kbuild test bot by adding ifdef of > CONFIG_KPROBE_EVENTS, and CONFIG_UPROBE_EVENTS. > > RFC v1 cover letter: > > This is to follow up the discussion over "new kprobe api" at Linux > Plumbers 2017: > > https://www.linuxplumbersconf.org/2017/ocw/proposals/4808 > > With current kernel, user space tools can only create/destroy [k,u]probes > with a text-based API (kprobe_events and uprobe_events in tracefs). This > approach relies on user space to clean up the [k,u]probe after using them. > However, this is not easy for user space to clean up properly. > > To solve this problem, we introduce a file descriptor based API. > Specifically, we extended perf_event_open to create [k,u]probe, and attach > this [k,u]probe to the file descriptor created by perf_event_open. These > [k,u]probe are associated with this file descriptor, so they are not > available in tracefs. > > We reuse large portion of existing trace_kprobe and trace_uprobe code. > Currently, the file descriptor API does not support arguments as the > text-based API does. This should not be a problem, as user of the file > decriptor based API read data through other methods (bpf, etc.). > > I also include a patch to to bcc, and a patch to man-page perf_even_open. > Please see the list below. A fork of bcc with this patch is also available > on github: > > https://github.com/liu-song-6/bcc/tree/perf_event_open > > Thanks, > Song > > man-pages patch: > perf_event_open.2: add type kprobe and uprobe > > bcc patch: > bcc: Try use new API to create [k,u]probe with perf_event_open > > kernel patches: > > Song Liu (6): > perf: prepare perf_event.h for new types perf_kprobe and perf_uprobe > perf: copy new perf_event.h to tools/include/uapi > perf: implement pmu perf_kprobe > perf: implement pmu perf_uprobe > bpf: add option for bpf_load.c to use perf_kprobe > bpf: add new test test_many_kprobe > > include/linux/trace_events.h | 8 ++ > include/uapi/linux/perf_event.h | 4 + > kernel/events/core.c | 131 +++- > kernel/trace/trace_event_perf.c | 102 +++ > kernel/trace/trace_kprobe.c | 91 +++-- > kernel/trace/trace_probe.h| 11 ++ > kernel/trace/trace_uprobe.c | 86 ++-- > samples/bpf/Makefile | 3 + > samples/bpf/bpf_load.c| 66 ++-- > samples/bpf/bpf_load.h| 14 +++ > samples/bpf/test_many_kprobe_user.c | 186 > ++ > tools/include/uapi/linux/perf_event.h | 4 + > 12 files changed, 677 insertions(+), 29 deletions(-) > create mode 100644 samples/bpf/test_many_kprobe_user.c > > -- > 2.9.5 Thank you for using the SPDX ids! For this: Acked-by: Philippe Ombredanne <pombreda...@nexb.com>
Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.
On Fri, Dec 1, 2017 at 9:56 PM, David Daney <dda...@caviumnetworks.com> wrote: > On 12/01/2017 12:41 PM, Philippe Ombredanne wrote: >> >> David, >> >> On Fri, Dec 1, 2017 at 9:01 PM, David Daney <dda...@caviumnetworks.com> >> wrote: >>> >>> On 12/01/2017 11:49 AM, Philippe Ombredanne wrote: >>>> >>>> >>>> David, Greg, >>>> >>>> On Fri, Dec 1, 2017 at 6:42 PM, David Daney <dda...@caviumnetworks.com> >>>> wrote: >>>>> >>>>> >>>>> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote: >>>> >>>> >>>> [...] >>>>>>>> >>>>>>>> >>>>>>>> --- /dev/null >>>>>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c >>>>>>>> @@ -0,0 +1,371 @@ >>>>>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>>>>> +/* >>>>>>>> + * Resource manager for Octeon. >>>>>>>> + * >>>>>>>> + * This file is subject to the terms and conditions of the GNU >>>>>>>> General >>>>>>>> Public >>>>>>>> + * License. See the file "COPYING" in the main directory of this >>>>>>>> archive >>>>>>>> + * for more details. >>>>>>>> + * >>>>>>>> + * Copyright (C) 2017 Cavium, Inc. >>>>>>>> + */ >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Since you nicely included an SPDX id, you would not need the >>>>>> boilerplate anymore. e.g. these can go alright? >>>>> >>>>> >>>>> >>>>> >>>>> They may not be strictly speaking necessary, but I don't think they >>>>> hurt >>>>> anything. Unless there is a requirement to strip out the license text, >>>>> we >>>>> would stick with it as is. >>>> >>>> >>>> >>>> I think the requirement is there and that would be much better for >>>> everyone: keeping both is redundant and does not bring any value, does >>>> it? Instead it kinda removes the benefits of having the SPDX id in the >>>> first place IMHO. >>>> >>>> Furthermore, as there have been already ~12K+ files cleaned up and >>>> still over 60K files to go, it would really nice if new files could >>>> adopt the new style: this way we will not have to revisit and repatch >>>> them in the future. >>>> >>> >>> I am happy to follow any style Greg would suggest. There doesn't seem to >>> be >>> much documentation about how this should be done yet. >> >> >> Thomas (tglx) has already submitted a first series of doc patches a >> few weeks ago. And AFAIK he might be working on posting the updates >> soon, whenever his real time clock yields a few cycles away from real >> time coding work ;) >> >> See also these discussions with Linus [1][2][3], Thomas[4] and Greg[5] >> on this and mostly related topics >> >> [1] https://lkml.org/lkml/2017/11/2/715 >> [2] https://lkml.org/lkml/2017/11/25/125 >> [3] https://lkml.org/lkml/2017/11/25/133 >> [4] https://lkml.org/lkml/2017/11/2/805 >> [5] https://lkml.org/lkml/2017/10/19/165 >> > > OK, you convinced me. > > Thanks, > David > No! Thank you to you: For doing real work on the kernel that makes my servers and laptops run, while I am nitpicking you on comments. -- Cordially Philippe Ombredanne
Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.
David, On Fri, Dec 1, 2017 at 9:01 PM, David Daney <dda...@caviumnetworks.com> wrote: > On 12/01/2017 11:49 AM, Philippe Ombredanne wrote: >> >> David, Greg, >> >> On Fri, Dec 1, 2017 at 6:42 PM, David Daney <dda...@caviumnetworks.com> >> wrote: >>> >>> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote: >> >> [...] >>>>>> >>>>>> --- /dev/null >>>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c >>>>>> @@ -0,0 +1,371 @@ >>>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>>> +/* >>>>>> + * Resource manager for Octeon. >>>>>> + * >>>>>> + * This file is subject to the terms and conditions of the GNU >>>>>> General >>>>>> Public >>>>>> + * License. See the file "COPYING" in the main directory of this >>>>>> archive >>>>>> + * for more details. >>>>>> + * >>>>>> + * Copyright (C) 2017 Cavium, Inc. >>>>>> + */ >>>> >>>> >>>> >>>> Since you nicely included an SPDX id, you would not need the >>>> boilerplate anymore. e.g. these can go alright? >>> >>> >>> >>> They may not be strictly speaking necessary, but I don't think they hurt >>> anything. Unless there is a requirement to strip out the license text, >>> we >>> would stick with it as is. >> >> >> I think the requirement is there and that would be much better for >> everyone: keeping both is redundant and does not bring any value, does >> it? Instead it kinda removes the benefits of having the SPDX id in the >> first place IMHO. >> >> Furthermore, as there have been already ~12K+ files cleaned up and >> still over 60K files to go, it would really nice if new files could >> adopt the new style: this way we will not have to revisit and repatch >> them in the future. >> > > I am happy to follow any style Greg would suggest. There doesn't seem to be > much documentation about how this should be done yet. Thomas (tglx) has already submitted a first series of doc patches a few weeks ago. And AFAIK he might be working on posting the updates soon, whenever his real time clock yields a few cycles away from real time coding work ;) See also these discussions with Linus [1][2][3], Thomas[4] and Greg[5] on this and mostly related topics [1] https://lkml.org/lkml/2017/11/2/715 [2] https://lkml.org/lkml/2017/11/25/125 [3] https://lkml.org/lkml/2017/11/25/133 [4] https://lkml.org/lkml/2017/11/2/805 [5] https://lkml.org/lkml/2017/10/19/165 -- Cordially Philippe Ombredanne
Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.
David, Greg, On Fri, Dec 1, 2017 at 6:42 PM, David Daney <dda...@caviumnetworks.com> wrote: > On 11/30/2017 11:53 PM, Philippe Ombredanne wrote: [...] >>>> --- /dev/null >>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c >>>> @@ -0,0 +1,371 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Resource manager for Octeon. >>>> + * >>>> + * This file is subject to the terms and conditions of the GNU General >>>> Public >>>> + * License. See the file "COPYING" in the main directory of this >>>> archive >>>> + * for more details. >>>> + * >>>> + * Copyright (C) 2017 Cavium, Inc. >>>> + */ >> >> >> Since you nicely included an SPDX id, you would not need the >> boilerplate anymore. e.g. these can go alright? > > > They may not be strictly speaking necessary, but I don't think they hurt > anything. Unless there is a requirement to strip out the license text, we > would stick with it as is. I think the requirement is there and that would be much better for everyone: keeping both is redundant and does not bring any value, does it? Instead it kinda removes the benefits of having the SPDX id in the first place IMHO. Furthermore, as there have been already ~12K+ files cleaned up and still over 60K files to go, it would really nice if new files could adopt the new style: this way we will not have to revisit and repatch them in the future. -- Cordially Philippe Ombredanne
Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.
Carlos, On Thu, Nov 30, 2017 at 11:53 PM, James Hogan <james.ho...@mips.com> wrote: > On Tue, Nov 28, 2017 at 04:55:35PM -0800, David Daney wrote: >> From: Carlos Munoz <cmu...@cavium.com> >> >> Add a global resource manager to manage tagged pointers within >> bootmem allocated memory. This is used by various functional >> blocks in the Octeon core like the FPA, Ethernet nexus, etc. >> >> Signed-off-by: Carlos Munoz <cmu...@cavium.com> >> Signed-off-by: Steven J. Hill <steven.h...@cavium.com> >> Signed-off-by: David Daney <david.da...@cavium.com> >> --- >> arch/mips/cavium-octeon/Makefile | 3 +- >> arch/mips/cavium-octeon/resource-mgr.c | 371 >> + >> arch/mips/include/asm/octeon/octeon.h | 18 ++ >> 3 files changed, 391 insertions(+), 1 deletion(-) >> create mode 100644 arch/mips/cavium-octeon/resource-mgr.c >> >> diff --git a/arch/mips/cavium-octeon/Makefile >> b/arch/mips/cavium-octeon/Makefile >> index 7c02e542959a..0a299ab8719f 100644 >> --- a/arch/mips/cavium-octeon/Makefile >> +++ b/arch/mips/cavium-octeon/Makefile >> @@ -9,7 +9,8 @@ >> # Copyright (C) 2005-2009 Cavium Networks >> # >> >> -obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o >> +obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o \ >> + resource-mgr.o > > Maybe put that on a separate line like below. > >> obj-y += dma-octeon.o >> obj-y += octeon-memcpy.o >> obj-y += executive/ >> diff --git a/arch/mips/cavium-octeon/resource-mgr.c >> b/arch/mips/cavium-octeon/resource-mgr.c >> new file mode 100644 >> index ..ca25fa953402 >> --- /dev/null >> +++ b/arch/mips/cavium-octeon/resource-mgr.c >> @@ -0,0 +1,371 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Resource manager for Octeon. >> + * >> + * This file is subject to the terms and conditions of the GNU General >> Public >> + * License. See the file "COPYING" in the main directory of this archive >> + * for more details. >> + * >> + * Copyright (C) 2017 Cavium, Inc. >> + */ Since you nicely included an SPDX id, you would not need the boilerplate anymore. e.g. these can go alright? >> + * This file is subject to the terms and conditions of the GNU General >> Public >> + * License. See the file "COPYING" in the main directory of this archive >> + * for more details. -- Cordially Philippe Ombredanne
Re: [PATCH v2 6/6] bpf: add new test test_many_kprobe
On Thu, Nov 30, 2017 at 2:44 AM, Song Liu <songliubrav...@fb.com> wrote:
> The test compares old text based kprobe API with PERF_TYPE_KPROBE.
>
> Here is a sample output of this test:
>
> Creating 1000 kprobes with text-based API takes 6.979683 seconds
> Cleaning 1000 kprobes with text-based API takes 84.897687 seconds
> Creating 1000 kprobes with PERF_TYPE_KPROBE (function name) takes 5.077558
> seconds
> Cleaning 1000 kprobes with PERF_TYPE_KPROBE (function name) takes 81.241354
> seconds
> Creating 1000 kprobes with PERF_TYPE_KPROBE (function addr) takes 5.218255
> seconds
> Cleaning 1000 kprobes with PERF_TYPE_KPROBE (function addr) takes 80.010731
> seconds
>
> Signed-off-by: Song Liu <songliubrav...@fb.com>
> Reviewed-by: Josef Bacik <jba...@fb.com>
> ---
> samples/bpf/Makefile| 3 +
> samples/bpf/bpf_load.c | 5 +-
> samples/bpf/bpf_load.h | 4 +
> samples/bpf/test_many_kprobe_user.c | 182
>
> 4 files changed, 191 insertions(+), 3 deletions(-)
> create mode 100644 samples/bpf/test_many_kprobe_user.c
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 9b4a66e..ec92f35 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -42,6 +42,7 @@ hostprogs-y += xdp_redirect
> hostprogs-y += xdp_redirect_map
> hostprogs-y += xdp_monitor
> hostprogs-y += syscall_tp
> +hostprogs-y += test_many_kprobe
>
> # Libbpf dependencies
> LIBBPF := ../../tools/lib/bpf/bpf.o
> @@ -87,6 +88,7 @@ xdp_redirect-objs := bpf_load.o $(LIBBPF)
> xdp_redirect_user.o
> xdp_redirect_map-objs := bpf_load.o $(LIBBPF) xdp_redirect_map_user.o
> xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o
> syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
> +test_many_kprobe-objs := bpf_load.o $(LIBBPF) test_many_kprobe_user.o
>
> # Tell kbuild to always build the programs
> always := $(hostprogs-y)
> @@ -172,6 +174,7 @@ HOSTLOADLIBES_xdp_redirect += -lelf
> HOSTLOADLIBES_xdp_redirect_map += -lelf
> HOSTLOADLIBES_xdp_monitor += -lelf
> HOSTLOADLIBES_syscall_tp += -lelf
> +HOSTLOADLIBES_test_many_kprobe += -lelf
>
> # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on
> cmdline:
> # make samples/bpf/ LLC=~/git/llvm/build/bin/llc
> CLANG=~/git/llvm/build/bin/clang
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 872510e..caba9bc 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -635,9 +635,8 @@ void read_trace_pipe(void)
> }
> }
>
> -#define MAX_SYMS 30
> -static struct ksym syms[MAX_SYMS];
> -static int sym_cnt;
> +struct ksym syms[MAX_SYMS];
> +int sym_cnt;
>
> static int ksym_cmp(const void *p1, const void *p2)
> {
> diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
> index e7a8a21..16bc263 100644
> --- a/samples/bpf/bpf_load.h
> +++ b/samples/bpf/bpf_load.h
> @@ -67,6 +67,10 @@ static inline __u64 ptr_to_u64(const void *ptr)
> return (__u64) (unsigned long) ptr;
> }
>
> +#define MAX_SYMS 30
> +extern struct ksym syms[MAX_SYMS];
> +extern int sym_cnt;
> +
> int load_kallsyms(void);
> struct ksym *ksym_search(long key);
> int set_link_xdp_fd(int ifindex, int fd, __u32 flags);
> diff --git a/samples/bpf/test_many_kprobe_user.c
> b/samples/bpf/test_many_kprobe_user.c
> new file mode 100644
> index 000..1f3ee07
> --- /dev/null
> +++ b/samples/bpf/test_many_kprobe_user.c
> @@ -0,0 +1,182 @@
> +/* Copyright (c) 2017 Facebook
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + */
I think an SPDX id would be better here e.g. just this may be?
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Facebook
It should be on the first line as requested by Linus and documented by
Thomas (tglx) and Greg (greg-kh). And it should use // comments.
See threads on these topics.
--
Cordially
Philippe Ombredanne
Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define
On Fri, Nov 17, 2017 at 3:19 PM, Greg Kroah-Hartman <gre...@linuxfoundation.org> wrote: > There is no need to #define the license of the driver, just put it in > the MODULE_LICENSE() line directly as a text string. > > This allows tools that check that the module license matches the source > code license to work properly, as there is no need to unwind the > unneeded dereference. > > Cc: "David S. Miller" <da...@davemloft.net> > Cc: Andreas Kemnade <andr...@kemnade.info> > Cc: Johan Hovold <jo...@kernel.org> > Reported-by: Philippe Ombredanne <pombreda...@nexb.com> > Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org> Reviewed-by: Philippe Ombredanne <pombreda...@nexb.com> -- Cordially Philippe Ombredanne
