Re: [PATCH v3 2/4] kconfig: regenerate *.c_shipped files after previous changes
On Mon, Nov 07, 2016 at 05:41:38PM -0500, Nicolas Pitre wrote: > On Mon, 7 Nov 2016, Josh Triplett wrote: > > > [snipping large patch] > > > > One suggestion that might make this patch easier to review: you might > > consider first regenerating the unchanged parser with Bison 3.0.4, then > > regenerating it again after the "imply" change. I think that'd > > eliminate quite a lot of noise in this patch. > > I tried that. This made two large patches instead of just one, both > equally obscure. > > So this patch stands on its own, containing changes that are > mechanically generated and therefore shouldn't require manual review. Fair enough. I hadn't expected that the changes from "imply" would still be huge.
Re: [PATCH v3 2/4] kconfig: regenerate *.c_shipped files after previous changes
[snipping large patch] One suggestion that might make this patch easier to review: you might consider first regenerating the unchanged parser with Bison 3.0.4, then regenerating it again after the "imply" change. I think that'd eliminate quite a lot of noise in this patch. - Josh Triplett
Re: [PATCH 0/4] make POSIX timers optional with some Kconfig help
On Wed, Oct 19, 2016 at 07:42:49PM -0400, Nicolas Pitre wrote: > Many embedded systems don't need the full POSIX timer support. > Configuring them out provides a nice kernel image size reduction. > > When POSIX timers are configured out, the PTP clock subsystem should be > left out as well. However a bunch of ethernet drivers currently *select* > the later in their Kconfig entries. Therefore some more work was needed > to break that hard dependency from those drivers without preventing their > usage altogether. > > Therefore this series also includes kconfig changes to implement a new > keyword to express some reverse dependencies like "select" does, named > "imply", and still allowing for the target config symbol to be disabled > if the user or a direct dependency says so. > > How to deal with the dependencies across three subsystems for potential > upstream merging needs to be figured out. This looks good to me, and I like the new "imply" approach. I'd still like to see a more general solution for reporting the use of compiled-out syscalls, but I don't think that needs to block this patch series. Reviewed-by: Josh Triplett <j...@joshtriplett.org>
Re: [PATCH 3/4] ptp_clock: allow for it to be optional
On Thu, Oct 20, 2016 at 04:06:02PM +0200, Richard Cochran wrote:
> On Wed, Oct 19, 2016 at 07:42:52PM -0400, Nicolas Pitre wrote:
> > +static inline void ptp_clock_event(struct ptp_clock *ptp,
> > + struct ptp_clock_event *event)
> > +{ (void)event; }
>
> Just out of curiosity, why do you need that cast?
>
> (I thought the kernel's gcc settings don't enable the warning for
> unused arguments.)
And if they did, wouldn't you need it for ptp as well?
Re: [PATCH 1/4] kconfig: introduce the "imply" keyword
On Thu, Oct 20, 2016 at 03:52:04PM +0100, Edward Cree wrote: > On 20/10/16 00:42, Nicolas Pitre wrote: > > diff --git a/Documentation/kbuild/kconfig-language.txt > > b/Documentation/kbuild/kconfig-language.txt > > index 069fcb3eef..c96127f648 100644 > > --- a/Documentation/kbuild/kconfig-language.txt > > +++ b/Documentation/kbuild/kconfig-language.txt > > @@ -113,6 +113,33 @@ applicable everywhere (see syntax). > > That will limit the usefulness but on the other hand avoid > > the illegal configurations all over. > > > > +- weak reverse dependencies: "imply" ["if" ] > > + This is similar to "select" as it enforces a lower limit on another > > + symbol except that the "implied" config symbol's value may still be > > + set to n from a direct dependency or with a visible prompt. > > + Given the following example: > > + > > + config FOO > > + tristate > > + imply BAZ > > + > > + config BAZ > > + tristate > > + depends on BAr > > + > > + The following values are possible: > > + > > + FOO BAR BAR's default choice for BAZ > Should the third column not be "BAZ's default"? > > + --- --- --- -- > > + n y n N/m/y > > + m y m M/y/n > > + y y y Y/n > > + y n * N > Also, I don't think having any FOO=y should preclude BAZ=m. Suppose both > FOO and FOO2 imply BAZ, FOO=y and FOO2=m. Then if BAZ-features are only > desired for driver FOO2, BAz=m makes sense. That's exactly the problem that motivated "imply" in the first place: while that's *possible*, it means the user needs to know that they're breaking BAZ support for driver FOO. In theory, someone could extend the UI to note the symbols with an "imply" for a given symbol and provide additional help for the implied symbol that explains the implications. In that case, it might make sense to allow the user to explicitly mark a symbol as 'm', with appropriate explanations of the implications. But in the absence of that, the simple solution seems like preventing 'm' for a symbol implied by a symbol marked as 'y'.
Re: [PATCH 1/2] ptp_clock: allow for it to be optional
On Mon, Sep 19, 2016 at 07:04:15PM +0200, Jiri Benc wrote:
> On Mon, 19 Sep 2016 10:10:21 -0400 (EDT), Nicolas Pitre wrote:
> > --- a/include/linux/ptp_clock_kernel.h
> > +++ b/include/linux/ptp_clock_kernel.h
> > @@ -207,7 +207,16 @@ int ptp_find_pin(struct ptp_clock *ptp,
> > #else
> > static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info
> > *info,
> >struct device *parent)
> > -{ return NULL; }
> > +{
> > + if (IS_MODULE(CONFIG_PTP_1588_CLOCK)) {
> > + pr_warn("%s is built-in while PTP clock subsystem is modular, "
> > + "PTP clock ignored\n", KBUILD_MODNAME);
> > + } else {
> > + pr_warn("ignoring PTP clock from %s as PTP clock subsystem "
> > + "is configured out\n", KBUILD_MODNAME);
> > + }
> > + return NULL;
> > +}
>
> I think the else part is not needed. If PTP is disabled, it is
> disabled, nobody should be surprised by that. Looks good otherwise.
This works, and it should compile away to nothing in the normal case.
But it does seem unfortunate that this can't happen at build time via
Kconfig. CCing linux-kbuild in case someone has an idea for how to fix
this.
Re: [PATCH] net: add sparse annotation to ptype_seq_start/stop
Paul E. McKenney wrote:
On Sun, Jan 20, 2008 at 03:21:46PM -0800, Stephen Hemminger wrote:
Get rid of some more sparse warnings.
Signed-off-by: Stephen Hemminger [EMAIL PROTECTED]
Adding Josh to CC -- is __acquires() case-insensitive? If not, this
needs to be __acquires(RCU) and __releases(RCU).
[...]
--- a/net/core/dev.c 2008-01-20 14:25:00.0 -0800
+++ b/net/core/dev.c 2008-01-20 14:25:48.0 -0800
@@ -2543,6 +2543,7 @@ static void *ptype_get_idx(loff_t pos)
}
static void *ptype_seq_start(struct seq_file *seq, loff_t *pos)
+__acquires(rcu)
{
rcu_read_lock();
return *pos ? ptype_get_idx(*pos - 1) : SEQ_START_TOKEN;
@@ -2578,6 +2579,7 @@ found:
}
static void ptype_seq_stop(struct seq_file *seq, void *v)
+__releases(rcu)
{
rcu_read_unlock();
}
At the moment, Sparse doesn't actually use the context expression. In
the ideal case when it does, all references to the same lock should
use a context expression which resolves to that lock (whatever that
may mean; hence why Sparse doesn't handle it yet). For mechanisms
like RCU without a lock variable, this will likely entail some sort of
fake lock expression, which again needs to match between all users of
the same mechanism. Like any expression in C, case matters; thus,
please match the existing references to RCU rather than rcu.
- Josh Triplett
signature.asc
Description: OpenPGP digital signature
[PATCH] Allow pktgen to work with loopback devices.
pktgen currently only works on network devices with type ARPHRD_ETHER. Add
support for the loopback device, type ARPHRD_LOOPBACK.
I've tested this on my system, using a modified pktgen.conf-1-1 with
s/eth1/lo/g, and it works fine; the network device statistics confirm packet
transmission and receipt.
Thanks to Sarah Bailey for discovering and tracking down the problem.
Signed-off-by: Josh Triplett [EMAIL PROTECTED]
---
I intentionally didn't change the error message not an ethernet device. For
the purposes of pktgen, loopback devices act like ethernet devices.
net/core/pktgen.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 74a9a32..6fc6f9d 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1930,7 +1930,7 @@ static struct net_device *pktgen_setup_dev(struct
pktgen_dev *pkt_dev)
printk(pktgen: no such netdevice: \%s\\n, pkt_dev-ifname);
goto out;
}
- if (odev-type != ARPHRD_ETHER) {
+ if (odev-type != ARPHRD_LOOPBACK odev-type != ARPHRD_ETHER) {
printk(pktgen: not an ethernet device: \%s\\n,
pkt_dev-ifname);
goto out_put;
--
1.5.0.2
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
