Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1475738 > unrolled thread

[PATCH 0/4] sparc: bpf_jit: Fine-tuning for bpf_jit_compile()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/4] sparc: bpf_jit: Fine-tuning for bpf_jit_compile() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-03 18:40 +0200
    [PATCH 1/4] sparc: bpf_jit: Use kmalloc_array() in bpf_jit_compile() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-03 18:40 +0200
      Re: [PATCH 1/4] sparc: bpf_jit: Use kmalloc_array() in bpf_jit_compile() Daniel Borkmann <daniel@iogearbox.net> - 2016-09-03 19:00 +0200
    [PATCH 4/4] sparc: bpf_jit: Rename jump labels in bpf_jit_compile() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-03 18:50 +0200
      Re: [PATCH 4/4] sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Daniel Borkmann <daniel@iogearbox.net> - 2016-09-03 19:00 +0200
        Re: [PATCH 4/4] sparc: bpf_jit: Rename jump labels in  bpf_jit_compile() David Miller <davem@davemloft.net> - 2016-09-04 08:20 +0200
          Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-04 09:00 +0200
            Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() David Miller <davem@davemloft.net> - 2016-09-04 09:00 +0200
              Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-04 09:40 +0200
                Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() David Miller <davem@davemloft.net> - 2016-09-04 09:40 +0200
                  Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-04 09:50 +0200
                Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Daniel Borkmann <daniel@iogearbox.net> - 2016-09-04 12:00 +0200
                  Re: Clarification for source code formatting around jump labels SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-04 16:00 +0200
                    Re: Clarification for source code formatting around jump labels Daniel Borkmann <daniel@iogearbox.net> - 2016-09-04 19:40 +0200
                  Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Jean Delvare <jdelvare@suse.de> - 2016-09-05 13:10 +0200
                    Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Peter Zijlstra <peterz@infradead.org> - 2016-09-05 13:40 +0200
                      Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Peter Zijlstra <peterz@infradead.org> - 2016-09-05 14:00 +0200
                        Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Jean Delvare <jdelvare@suse.de> - 2016-09-06 16:40 +0200
                          Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Peter Zijlstra <peterz@infradead.org> - 2016-09-06 16:50 +0200
                            Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Joe Perches <joe@perches.com> - 2016-09-06 17:30 +0200
                            Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Jean Delvare <jdelvare@suse.de> - 2016-09-07 14:40 +0200
                      Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile() Jean Delvare <jdelvare@suse.de> - 2016-09-05 14:00 +0200
    [PATCH 3/4] sparc: bpf_jit: Avoid assignment for "flen" if BPF JIT is  disabled SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-03 18:50 +0200
      Re: [PATCH 3/4] sparc: bpf_jit: Avoid assignment for "flen" if BPF  JIT is disabled Daniel Borkmann <daniel@iogearbox.net> - 2016-09-03 19:00 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1478252 — Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile()

Hi Peter,

On Tue, 6 Sep 2016 16:47:56 +0200, Peter Zijlstra wrote:
> On Tue, Sep 06, 2016 at 04:34:13PM +0200, Jean Delvare wrote:
> > > [diff "default"]
> > >         xfuncname = "^[[:alpha:]$_].*[^:]$"
> > 
> > OK, I see. As mentioned somewhere else, it fails for labels which have
> > comments. 
> 
> Heh, There's labels that have comments?

Yes, 43.

> > My worry is that you recommending it to contributors on a individual
> > and opportunity basis, doesn't scale. Basing coding style
> > recommendations on a personal quirk doesn't strike me as the best idea
> > ever in the long run.
> 
> Don't care too much, I simply will not take any patch that adds stupid
> spaces :-)
> 
> > While looking at the syntax of your example, I have found something
> > which looks more promising. git already has predefined xfuncname
> > definitions for various languages, including C. These can be enabled
> > based on file name patterns via gitattributes. The
> > following .gitattribute file placed at the root of the kernel source
> > tree achieves what you want:
> > 
> > *.c   diff=cpp
> > *.h   diff=cpp
> > 
> > The major difference between git config and gitattributes is that the
> > latter can be part of the project itself, just like gitignore. So we
> > could just push that .gitattribute file upstream, and then labels
> > without leading spaces would no longer be a problem, at least within
> > git.
> 
> Works for me,

OK, I'll send a patch now.

> and last time this came up Linus agreed with the
> "whitespace before labels is stupid" thing. Although I cannot find a
> link to that just now.

Murphy's law applies, you can never find links again the day you
desperately need them. If you ever get your hands on that one again,
please let me know, I really would like to read that post.

> > It would still be a problem for me as an inveterate quilt user,
> 
> Add the below to your .quiltrc or environment:
> 
> QUILT_DIFF_OPTS="-F ^[[:alpha:]\$_].*[^:]\$"

I didn't know this option existed, thanks for the pointer.

Now I'm sure I won't try to get the behavior of GNU diff option -p
changed, as I think I know what the answer would be.

> Same caveat about labels with comments, but then I'd not take a patch
> doing that in the first place.

I'll improve the regular expression if I ever have to (I don't think a
function declaration can have a colon anywhere?), but I'm happy if it
works in 99.9 % of the cases, thank sagain.

-- 
Jean Delvare
SUSE L3 Support

[toc] | [prev] | [next] | [standalone]

#1476352 — Re: sparc: bpf_jit: Rename jump labels in bpf_jit_compile()

On Mon, 5 Sep 2016 13:37:04 +0200, Peter Zijlstra wrote:
> On Mon, Sep 05, 2016 at 01:07:37PM +0200, Jean Delvare wrote:
> > Now I see in http://patchwork.ozlabs.org/patch/664966/ that Peter
> > Zijlstra reportedly changed the behavior of "diff -p" so that it
> > handles unindented C labels nicely. If this actually happens, it could
> > change my point of view. However I can't find this commit in upstream
> > diffutils. Peter, can you please clarify the situation? Is it just a
> > local hack on your own instance of "diff"?
> 
> I have it in my local .gitconfig, and recommend it to people who send me
> patches.

What does it look like, please?

> I've never tried to get diffutils fixed, although maybe I should.

If you don't want one-space-indented labels to become the norm, then
yes you should.

-- 
Jean Delvare
SUSE L3 Support

[toc] | [prev] | [next] | [standalone]

#1475746 — [PATCH 3/4] sparc: bpf_jit: Avoid assignment for "flen" if BPF JIT is disabled

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Sep 2016 18:00:03 +0200

Move the assignment for the local variable "flen" a bit at the beginning
so that it will only be performed if BPF JIT is enabled for this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/sparc/net/bpf_jit_comp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/sparc/net/bpf_jit_comp.c b/arch/sparc/net/bpf_jit_comp.c
index a927470..a6b6e29 100644
--- a/arch/sparc/net/bpf_jit_comp.c
+++ b/arch/sparc/net/bpf_jit_comp.c
@@ -365,13 +365,14 @@ void bpf_jit_compile(struct bpf_prog *fp)
 	unsigned int cleanup_addr, proglen, oldproglen;
 	u32 temp[8], *prog, *func, seen, pass;
 	const struct sock_filter *filter;
-	int i, flen = fp->len, pc_ret0;
+	int i, flen, pc_ret0;
 	unsigned int *addrs;
 	void *image;
 
 	if (!bpf_jit_enable)
 		return;
 
+	flen = fp->len;
 	addrs = kmalloc_array(flen, sizeof(*addrs), GFP_KERNEL);
 	if (addrs == NULL)
 		return;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1475751 — Re: [PATCH 3/4] sparc: bpf_jit: Avoid assignment for "flen" if BPF JIT is disabled

On 09/03/2016 06:40 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 3 Sep 2016 18:00:03 +0200
>
> Move the assignment for the local variable "flen" a bit at the beginning
> so that it will only be performed if BPF JIT is enabled for this function.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Why is that a particular issue (did you measure some improvement when
you say "fine-tuning" as per subject) ?

That's all not in critical path here by the way, same for patch 2.

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web