| Message ID | 7cc162af77ba918eb3ecd26ec9e7824bc44b1fae.1641468127.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | powerpc/bpf: Some fixes and updates | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-PR | fail | merge-conflict |
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Thu, Jan 06, 2022 at 05:15:07PM +0530, Naveen N. Rao wrote: > These instructions are updated after the initial JIT, so redo codegen > during the extra pass. Rename bpf_jit_fixup_subprog_calls() to clarify > that this is more than just subprog calls. > > Fixes: 69c087ba6225b5 ("bpf: Add bpf_for_each_map_elem() helper") > Cc: stable@vger.kernel.org # v5.15 > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> Tested-by: Jiri Olsa <jolsa@redhat.com> thanks, jirka > --- > arch/powerpc/net/bpf_jit_comp.c | 29 +++++++++++++++++++++++------ > arch/powerpc/net/bpf_jit_comp32.c | 6 ++++++ > arch/powerpc/net/bpf_jit_comp64.c | 7 ++++++- > 3 files changed, 35 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c > index d6ffdd0f2309d0..56dd1f4e3e4447 100644 > --- a/arch/powerpc/net/bpf_jit_comp.c > +++ b/arch/powerpc/net/bpf_jit_comp.c > @@ -23,15 +23,15 @@ static void bpf_jit_fill_ill_insns(void *area, unsigned int size) > memset32(area, BREAKPOINT_INSTRUCTION, size / 4); > } > > -/* Fix the branch target addresses for subprog calls */ > -static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, > - struct codegen_context *ctx, u32 *addrs) > +/* Fix updated addresses (for subprog calls, ldimm64, et al) during extra pass */ > +static int bpf_jit_fixup_addresses(struct bpf_prog *fp, u32 *image, > + struct codegen_context *ctx, u32 *addrs) > { > const struct bpf_insn *insn = fp->insnsi; > bool func_addr_fixed; > u64 func_addr; > u32 tmp_idx; > - int i, ret; > + int i, j, ret; > > for (i = 0; i < fp->len; i++) { > /* > @@ -66,6 +66,23 @@ static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, > * of the JITed sequence remains unchanged. > */ > ctx->idx = tmp_idx; > + } else if (insn[i].code == (BPF_LD | BPF_IMM | BPF_DW)) { > + tmp_idx = ctx->idx; > + ctx->idx = addrs[i] / 4; > +#ifdef CONFIG_PPC32 > + PPC_LI32(ctx->b2p[insn[i].dst_reg] - 1, (u32)insn[i + 1].imm); > + PPC_LI32(ctx->b2p[insn[i].dst_reg], (u32)insn[i].imm); > + for (j = ctx->idx - addrs[i] / 4; j < 4; j++) > + EMIT(PPC_RAW_NOP()); > +#else > + func_addr = ((u64)(u32)insn[i].imm) | (((u64)(u32)insn[i + 1].imm) << 32); > + PPC_LI64(b2p[insn[i].dst_reg], func_addr); > + /* overwrite rest with nops */ > + for (j = ctx->idx - addrs[i] / 4; j < 5; j++) > + EMIT(PPC_RAW_NOP()); > +#endif > + ctx->idx = tmp_idx; > + i++; > } > } > > @@ -200,13 +217,13 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp) > /* > * Do not touch the prologue and epilogue as they will remain > * unchanged. Only fix the branch target address for subprog > - * calls in the body. > + * calls in the body, and ldimm64 instructions. > * > * This does not change the offsets and lengths of the subprog > * call instruction sequences and hence, the size of the JITed > * image as well. > */ > - bpf_jit_fixup_subprog_calls(fp, code_base, &cgctx, addrs); > + bpf_jit_fixup_addresses(fp, code_base, &cgctx, addrs); > > /* There is no need to perform the usual passes. */ > goto skip_codegen_passes; > diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c > index 997a47fa615b30..2258d3886d02ec 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -293,6 +293,8 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > bool func_addr_fixed; > u64 func_addr; > u32 true_cond; > + u32 tmp_idx; > + int j; > > /* > * addrs[] maps a BPF bytecode address into a real offset from > @@ -908,8 +910,12 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > * 16 byte instruction that uses two 'struct bpf_insn' > */ > case BPF_LD | BPF_IMM | BPF_DW: /* dst = (u64) imm */ > + tmp_idx = ctx->idx; > PPC_LI32(dst_reg_h, (u32)insn[i + 1].imm); > PPC_LI32(dst_reg, (u32)insn[i].imm); > + /* padding to allow full 4 instructions for later patching */ > + for (j = ctx->idx - tmp_idx; j < 4; j++) > + EMIT(PPC_RAW_NOP()); > /* Adjust for two bpf instructions */ > addrs[++i] = ctx->idx * 4; > break; > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c > index 472d4a551945dd..3d018ecc475b2b 100644 > --- a/arch/powerpc/net/bpf_jit_comp64.c > +++ b/arch/powerpc/net/bpf_jit_comp64.c > @@ -319,6 +319,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > u64 imm64; > u32 true_cond; > u32 tmp_idx; > + int j; > > /* > * addrs[] maps a BPF bytecode address into a real offset from > @@ -848,9 +849,13 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > case BPF_LD | BPF_IMM | BPF_DW: /* dst = (u64) imm */ > imm64 = ((u64)(u32) insn[i].imm) | > (((u64)(u32) insn[i+1].imm) << 32); > + tmp_idx = ctx->idx; > + PPC_LI64(dst_reg, imm64); > + /* padding to allow full 5 instructions for later patching */ > + for (j = ctx->idx - tmp_idx; j < 5; j++) > + EMIT(PPC_RAW_NOP()); > /* Adjust for two bpf instructions */ > addrs[++i] = ctx->idx * 4; > - PPC_LI64(dst_reg, imm64); > break; > > /* > -- > 2.34.1 >
Le 06/01/2022 à 12:45, Naveen N. Rao a écrit : > These instructions are updated after the initial JIT, so redo codegen > during the extra pass. Rename bpf_jit_fixup_subprog_calls() to clarify > that this is more than just subprog calls. > > Fixes: 69c087ba6225b5 ("bpf: Add bpf_for_each_map_elem() helper") > Cc: stable@vger.kernel.org # v5.15 > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > arch/powerpc/net/bpf_jit_comp.c | 29 +++++++++++++++++++++++------ > arch/powerpc/net/bpf_jit_comp32.c | 6 ++++++ > arch/powerpc/net/bpf_jit_comp64.c | 7 ++++++- > 3 files changed, 35 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c > index d6ffdd0f2309d0..56dd1f4e3e4447 100644 > --- a/arch/powerpc/net/bpf_jit_comp.c > +++ b/arch/powerpc/net/bpf_jit_comp.c > @@ -23,15 +23,15 @@ static void bpf_jit_fill_ill_insns(void *area, unsigned int size) > memset32(area, BREAKPOINT_INSTRUCTION, size / 4); > } > > -/* Fix the branch target addresses for subprog calls */ > -static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, > - struct codegen_context *ctx, u32 *addrs) > +/* Fix updated addresses (for subprog calls, ldimm64, et al) during extra pass */ > +static int bpf_jit_fixup_addresses(struct bpf_prog *fp, u32 *image, > + struct codegen_context *ctx, u32 *addrs) > { > const struct bpf_insn *insn = fp->insnsi; > bool func_addr_fixed; > u64 func_addr; > u32 tmp_idx; > - int i, ret; > + int i, j, ret; > > for (i = 0; i < fp->len; i++) { > /* > @@ -66,6 +66,23 @@ static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, > * of the JITed sequence remains unchanged. > */ > ctx->idx = tmp_idx; > + } else if (insn[i].code == (BPF_LD | BPF_IMM | BPF_DW)) { > + tmp_idx = ctx->idx; > + ctx->idx = addrs[i] / 4; > +#ifdef CONFIG_PPC32 > + PPC_LI32(ctx->b2p[insn[i].dst_reg] - 1, (u32)insn[i + 1].imm); > + PPC_LI32(ctx->b2p[insn[i].dst_reg], (u32)insn[i].imm); > + for (j = ctx->idx - addrs[i] / 4; j < 4; j++) > + EMIT(PPC_RAW_NOP()); > +#else > + func_addr = ((u64)(u32)insn[i].imm) | (((u64)(u32)insn[i + 1].imm) << 32); > + PPC_LI64(b2p[insn[i].dst_reg], func_addr); > + /* overwrite rest with nops */ > + for (j = ctx->idx - addrs[i] / 4; j < 5; j++) > + EMIT(PPC_RAW_NOP()); > +#endif #ifdefs should be avoided as much as possible. Here it seems we could easily do an if (IS_ENABLED(CONFIG_PPC32)) { } else { } And it looks like the CONFIG_PPC64 alternative would in fact also work on PPC32, wouldn't it ? > + ctx->idx = tmp_idx; > + i++; > } > } > > @@ -200,13 +217,13 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp) > /* > * Do not touch the prologue and epilogue as they will remain > * unchanged. Only fix the branch target address for subprog > - * calls in the body. > + * calls in the body, and ldimm64 instructions. > * > * This does not change the offsets and lengths of the subprog > * call instruction sequences and hence, the size of the JITed > * image as well. > */ > - bpf_jit_fixup_subprog_calls(fp, code_base, &cgctx, addrs); > + bpf_jit_fixup_addresses(fp, code_base, &cgctx, addrs); > > /* There is no need to perform the usual passes. */ > goto skip_codegen_passes; > diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c > index 997a47fa615b30..2258d3886d02ec 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -293,6 +293,8 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > bool func_addr_fixed; > u64 func_addr; > u32 true_cond; > + u32 tmp_idx; > + int j; > > /* > * addrs[] maps a BPF bytecode address into a real offset from > @@ -908,8 +910,12 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > * 16 byte instruction that uses two 'struct bpf_insn' > */ > case BPF_LD | BPF_IMM | BPF_DW: /* dst = (u64) imm */ > + tmp_idx = ctx->idx; > PPC_LI32(dst_reg_h, (u32)insn[i + 1].imm); > PPC_LI32(dst_reg, (u32)insn[i].imm); > + /* padding to allow full 4 instructions for later patching */ > + for (j = ctx->idx - tmp_idx; j < 4; j++) > + EMIT(PPC_RAW_NOP()); > /* Adjust for two bpf instructions */ > addrs[++i] = ctx->idx * 4; > break; > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c > index 472d4a551945dd..3d018ecc475b2b 100644 > --- a/arch/powerpc/net/bpf_jit_comp64.c > +++ b/arch/powerpc/net/bpf_jit_comp64.c > @@ -319,6 +319,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > u64 imm64; > u32 true_cond; > u32 tmp_idx; > + int j; > > /* > * addrs[] maps a BPF bytecode address into a real offset from > @@ -848,9 +849,13 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > case BPF_LD | BPF_IMM | BPF_DW: /* dst = (u64) imm */ > imm64 = ((u64)(u32) insn[i].imm) | > (((u64)(u32) insn[i+1].imm) << 32); > + tmp_idx = ctx->idx; > + PPC_LI64(dst_reg, imm64); > + /* padding to allow full 5 instructions for later patching */ > + for (j = ctx->idx - tmp_idx; j < 5; j++) > + EMIT(PPC_RAW_NOP()); > /* Adjust for two bpf instructions */ > addrs[++i] = ctx->idx * 4; > - PPC_LI64(dst_reg, imm64); > break; > > /*
Christophe Leroy wrote: > > > Le 06/01/2022 à 12:45, Naveen N. Rao a écrit : >> These instructions are updated after the initial JIT, so redo codegen >> during the extra pass. Rename bpf_jit_fixup_subprog_calls() to clarify >> that this is more than just subprog calls. >> >> Fixes: 69c087ba6225b5 ("bpf: Add bpf_for_each_map_elem() helper") >> Cc: stable@vger.kernel.org # v5.15 >> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> >> --- >> arch/powerpc/net/bpf_jit_comp.c | 29 +++++++++++++++++++++++------ >> arch/powerpc/net/bpf_jit_comp32.c | 6 ++++++ >> arch/powerpc/net/bpf_jit_comp64.c | 7 ++++++- >> 3 files changed, 35 insertions(+), 7 deletions(-) >> >> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c >> index d6ffdd0f2309d0..56dd1f4e3e4447 100644 >> --- a/arch/powerpc/net/bpf_jit_comp.c >> +++ b/arch/powerpc/net/bpf_jit_comp.c >> @@ -23,15 +23,15 @@ static void bpf_jit_fill_ill_insns(void *area, unsigned int size) >> memset32(area, BREAKPOINT_INSTRUCTION, size / 4); >> } >> >> -/* Fix the branch target addresses for subprog calls */ >> -static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, >> - struct codegen_context *ctx, u32 *addrs) >> +/* Fix updated addresses (for subprog calls, ldimm64, et al) during extra pass */ >> +static int bpf_jit_fixup_addresses(struct bpf_prog *fp, u32 *image, >> + struct codegen_context *ctx, u32 *addrs) >> { >> const struct bpf_insn *insn = fp->insnsi; >> bool func_addr_fixed; >> u64 func_addr; >> u32 tmp_idx; >> - int i, ret; >> + int i, j, ret; >> >> for (i = 0; i < fp->len; i++) { >> /* >> @@ -66,6 +66,23 @@ static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, >> * of the JITed sequence remains unchanged. >> */ >> ctx->idx = tmp_idx; >> + } else if (insn[i].code == (BPF_LD | BPF_IMM | BPF_DW)) { >> + tmp_idx = ctx->idx; >> + ctx->idx = addrs[i] / 4; >> +#ifdef CONFIG_PPC32 >> + PPC_LI32(ctx->b2p[insn[i].dst_reg] - 1, (u32)insn[i + 1].imm); >> + PPC_LI32(ctx->b2p[insn[i].dst_reg], (u32)insn[i].imm); >> + for (j = ctx->idx - addrs[i] / 4; j < 4; j++) >> + EMIT(PPC_RAW_NOP()); >> +#else >> + func_addr = ((u64)(u32)insn[i].imm) | (((u64)(u32)insn[i + 1].imm) << 32); >> + PPC_LI64(b2p[insn[i].dst_reg], func_addr); >> + /* overwrite rest with nops */ >> + for (j = ctx->idx - addrs[i] / 4; j < 5; j++) >> + EMIT(PPC_RAW_NOP()); >> +#endif > > #ifdefs should be avoided as much as possible. > > Here it seems we could easily do an > > if (IS_ENABLED(CONFIG_PPC32)) { > } else { > } > > And it looks like the CONFIG_PPC64 alternative would in fact also work > on PPC32, wouldn't it ? We never implemented PPC_LI64() for ppc32: /linux/arch/powerpc/net/bpf_jit_comp.c: In function 'bpf_jit_fixup_addresses': /linux/arch/powerpc/net/bpf_jit_comp.c:81:5: error: this decimal constant is unsigned only in ISO C90 [-Werror] 81 | PPC_LI64(b2p[insn[i].dst_reg], func_addr); | ^~~~~~~~ /linux/arch/powerpc/net/bpf_jit_comp.c:81:5: error: this decimal constant is unsigned only in ISO C90 [-Werror] In file included from /linux/arch/powerpc/net/bpf_jit_comp.c:19: /linux/arch/powerpc/net/bpf_jit.h:78:40: error: right shift count >= width of type [-Werror=shift-count-overflow] 78 | EMIT(PPC_RAW_LI(d, ((uintptr_t)(i) >> 32) & \ | ^~ We should move that out from bpf_jit.h - Naveen
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index d6ffdd0f2309d0..56dd1f4e3e4447 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -23,15 +23,15 @@ static void bpf_jit_fill_ill_insns(void *area, unsigned int size) memset32(area, BREAKPOINT_INSTRUCTION, size / 4); } -/* Fix the branch target addresses for subprog calls */ -static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, - struct codegen_context *ctx, u32 *addrs) +/* Fix updated addresses (for subprog calls, ldimm64, et al) during extra pass */ +static int bpf_jit_fixup_addresses(struct bpf_prog *fp, u32 *image, + struct codegen_context *ctx, u32 *addrs) { const struct bpf_insn *insn = fp->insnsi; bool func_addr_fixed; u64 func_addr; u32 tmp_idx; - int i, ret; + int i, j, ret; for (i = 0; i < fp->len; i++) { /* @@ -66,6 +66,23 @@ static int bpf_jit_fixup_subprog_calls(struct bpf_prog *fp, u32 *image, * of the JITed sequence remains unchanged. */ ctx->idx = tmp_idx; + } else if (insn[i].code == (BPF_LD | BPF_IMM | BPF_DW)) { + tmp_idx = ctx->idx; + ctx->idx = addrs[i] / 4; +#ifdef CONFIG_PPC32 + PPC_LI32(ctx->b2p[insn[i].dst_reg] - 1, (u32)insn[i + 1].imm); + PPC_LI32(ctx->b2p[insn[i].dst_reg], (u32)insn[i].imm); + for (j = ctx->idx - addrs[i] / 4; j < 4; j++) + EMIT(PPC_RAW_NOP()); +#else + func_addr = ((u64)(u32)insn[i].imm) | (((u64)(u32)insn[i + 1].imm) << 32); + PPC_LI64(b2p[insn[i].dst_reg], func_addr); + /* overwrite rest with nops */ + for (j = ctx->idx - addrs[i] / 4; j < 5; j++) + EMIT(PPC_RAW_NOP()); +#endif + ctx->idx = tmp_idx; + i++; } } @@ -200,13 +217,13 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp) /* * Do not touch the prologue and epilogue as they will remain * unchanged. Only fix the branch target address for subprog - * calls in the body. + * calls in the body, and ldimm64 instructions. * * This does not change the offsets and lengths of the subprog * call instruction sequences and hence, the size of the JITed * image as well. */ - bpf_jit_fixup_subprog_calls(fp, code_base, &cgctx, addrs); + bpf_jit_fixup_addresses(fp, code_base, &cgctx, addrs); /* There is no need to perform the usual passes. */ goto skip_codegen_passes; diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c index 997a47fa615b30..2258d3886d02ec 100644 --- a/arch/powerpc/net/bpf_jit_comp32.c +++ b/arch/powerpc/net/bpf_jit_comp32.c @@ -293,6 +293,8 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * bool func_addr_fixed; u64 func_addr; u32 true_cond; + u32 tmp_idx; + int j; /* * addrs[] maps a BPF bytecode address into a real offset from @@ -908,8 +910,12 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * * 16 byte instruction that uses two 'struct bpf_insn' */ case BPF_LD | BPF_IMM | BPF_DW: /* dst = (u64) imm */ + tmp_idx = ctx->idx; PPC_LI32(dst_reg_h, (u32)insn[i + 1].imm); PPC_LI32(dst_reg, (u32)insn[i].imm); + /* padding to allow full 4 instructions for later patching */ + for (j = ctx->idx - tmp_idx; j < 4; j++) + EMIT(PPC_RAW_NOP()); /* Adjust for two bpf instructions */ addrs[++i] = ctx->idx * 4; break; diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c index 472d4a551945dd..3d018ecc475b2b 100644 --- a/arch/powerpc/net/bpf_jit_comp64.c +++ b/arch/powerpc/net/bpf_jit_comp64.c @@ -319,6 +319,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * u64 imm64; u32 true_cond; u32 tmp_idx; + int j; /* * addrs[] maps a BPF bytecode address into a real offset from @@ -848,9 +849,13 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * case BPF_LD | BPF_IMM | BPF_DW: /* dst = (u64) imm */ imm64 = ((u64)(u32) insn[i].imm) | (((u64)(u32) insn[i+1].imm) << 32); + tmp_idx = ctx->idx; + PPC_LI64(dst_reg, imm64); + /* padding to allow full 5 instructions for later patching */ + for (j = ctx->idx - tmp_idx; j < 5; j++) + EMIT(PPC_RAW_NOP()); /* Adjust for two bpf instructions */ addrs[++i] = ctx->idx * 4; - PPC_LI64(dst_reg, imm64); break; /*
These instructions are updated after the initial JIT, so redo codegen during the extra pass. Rename bpf_jit_fixup_subprog_calls() to clarify that this is more than just subprog calls. Fixes: 69c087ba6225b5 ("bpf: Add bpf_for_each_map_elem() helper") Cc: stable@vger.kernel.org # v5.15 Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- arch/powerpc/net/bpf_jit_comp.c | 29 +++++++++++++++++++++++------ arch/powerpc/net/bpf_jit_comp32.c | 6 ++++++ arch/powerpc/net/bpf_jit_comp64.c | 7 ++++++- 3 files changed, 35 insertions(+), 7 deletions(-)