Re: [PATCH v2] bpf: s390: add JIT support for multi-function programs

[Ilya Leoshkevich <iii@linux.ibm.com>]

> Am 27.08.2019 um 18:39 schrieb Ilya Leoshkevich <iii@linux.ibm.com>:
> 
>> Am 27.08.2019 um 16:53 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
>> 
>> @@ -1316,7 +1327,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>> {
>> 	struct bpf_prog *tmp, *orig_fp = fp;
>> 	struct bpf_binary_header *header;
>> +	struct s390_jit_data *jit_data;
>> 	bool tmp_blinded = false;
>> +	bool extra_pass = false;
>> 	struct bpf_jit jit;
>> 	int pass;
>> 
>> @@ -1335,6 +1348,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>> 		fp = tmp;
>> 	}
>> 
>> +	jit_data = fp->aux->jit_data;
>> +	if (!jit_data) {
>> +		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
>> +		if (!jit_data) {
>> +			fp = orig_fp;
>> +			goto out;
>> +		}
>> +		fp->aux->jit_data = jit_data;
>> +	}
>> +	if (jit_data->ctx.addrs) {
>> +		jit = jit_data->ctx;
>> +		header = jit_data->header;
>> +		extra_pass = true;
>> +		goto skip_init_ctx;
>> +	}
>> +
> 
> I've noticed that I'm getting the following warning, presumably because
> of the added goto skip_init_ctx:
> 
> linux/arch/s390/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':
> linux/arch/s390/net/bpf_jit_comp.c:1406:3: warning: 'pass' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
> 
> Maybe set the initial value of pass to 1?

... or save pass in s390_jit_data and do the following?

	pass = jit_data->pass + 1;
	goto skip_init_ctx;

I've also noticed that

	$ git show --format=email | scripts/checkpatch.pl --strict

complains - could you please fix these warnings?


I've finally reviewed the actual logic of the patch, and it looks good
to me.  The integration with the common code is somewhat involved, but
it follows the established architecture.