Message ID | 1542547284-4115-1-git-send-email-yanjun.zhu@oracle.com (mailing list archive) |
---|---|
State | Rejected |
Delegated to: | Jason Gunthorpe |
Headers | show |
Series | [1/1] IB/rxe: use kvmalloc instead of kmalloc | expand |
On Sun, Nov 18, 2018 at 08:21:24AM -0500, Yanjun Zhu wrote: > Attempt to allocate physically contiguous memory, but upon > failure, fall back to non-contiguous (vmalloc) allocation. > > Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> > --- > drivers/infiniband/sw/rxe/rxe_cq.c | 2 +- > drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++-- > drivers/infiniband/sw/rxe/rxe_queue.c | 6 +++--- > drivers/infiniband/sw/rxe/rxe_srq.c | 2 +- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c > index a57276f2cb84..b85ca024fa58 100644 > --- a/drivers/infiniband/sw/rxe/rxe_cq.c > +++ b/drivers/infiniband/sw/rxe/rxe_cq.c > @@ -98,7 +98,7 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe, > cq->queue->buf, cq->queue->buf_size, &cq->queue->ip); > if (err) { > vfree(cq->queue->buf); > - kfree(cq->queue); > + kvfree(cq->queue); But queue is created with kmalloc, what am i missing here? Is this patch based on some other patch that is not yet in for-next? > return err; > } > > diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c > index b9710907dac2..fd233f93ac1d 100644 > --- a/drivers/infiniband/sw/rxe/rxe_qp.c > +++ b/drivers/infiniband/sw/rxe/rxe_qp.c > @@ -259,7 +259,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, > > if (err) { > vfree(qp->sq.queue->buf); > - kfree(qp->sq.queue); > + kvfree(qp->sq.queue); > return err; > } > > @@ -312,7 +312,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp, > &qp->rq.queue->ip); > if (err) { > vfree(qp->rq.queue->buf); > - kfree(qp->rq.queue); > + kvfree(qp->rq.queue); > return err; > } > } > diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c > index f84ab4469261..e811a655b286 100644 > --- a/drivers/infiniband/sw/rxe/rxe_queue.c > +++ b/drivers/infiniband/sw/rxe/rxe_queue.c > @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, > if (*num_elem < 0) > goto err1; > > - q = kmalloc(sizeof(*q), GFP_KERNEL); > + q = kvmalloc(sizeof(*q), GFP_KERNEL); > if (!q) > goto err1; > > @@ -174,7 +174,7 @@ int rxe_queue_resize(struct rxe_queue *q, > new_q->buf_size, &new_q->ip); > if (err) { > vfree(new_q->buf); > - kfree(new_q); > + kvfree(new_q); > goto err1; > } > > @@ -208,5 +208,5 @@ void rxe_queue_cleanup(struct rxe_queue *q) > else > vfree(q->buf); > > - kfree(q); > + kvfree(q); > } > diff --git a/drivers/infiniband/sw/rxe/rxe_srq.c b/drivers/infiniband/sw/rxe/rxe_srq.c > index c41a5fee81f7..c9969d157b87 100644 > --- a/drivers/infiniband/sw/rxe/rxe_srq.c > +++ b/drivers/infiniband/sw/rxe/rxe_srq.c > @@ -132,7 +132,7 @@ int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq, > q->buf_size, &q->ip); > if (err) { > vfree(q->buf); > - kfree(q); > + kvfree(q); > return err; > } > > -- > 2.17.1 >
On 2018/11/18 21:35, Yuval Shaia wrote: > On Sun, Nov 18, 2018 at 08:21:24AM -0500, Yanjun Zhu wrote: >> Attempt to allocate physically contiguous memory, but upon >> failure, fall back to non-contiguous (vmalloc) allocation. >> >> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> >> --- >> drivers/infiniband/sw/rxe/rxe_cq.c | 2 +- >> drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++-- >> drivers/infiniband/sw/rxe/rxe_queue.c | 6 +++--- >> drivers/infiniband/sw/rxe/rxe_srq.c | 2 +- >> 4 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c >> index a57276f2cb84..b85ca024fa58 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_cq.c >> +++ b/drivers/infiniband/sw/rxe/rxe_cq.c >> @@ -98,7 +98,7 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe, >> cq->queue->buf, cq->queue->buf_size, &cq->queue->ip); >> if (err) { >> vfree(cq->queue->buf); >> - kfree(cq->queue); >> + kvfree(cq->queue); > But queue is created with kmalloc, what am i missing here? Now I use kvmalloc to create queue. Zhu Yanjun > > Is this patch based on some other patch that is not yet in for-next? > >> return err; >> } >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c >> index b9710907dac2..fd233f93ac1d 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_qp.c >> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c >> @@ -259,7 +259,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, >> >> if (err) { >> vfree(qp->sq.queue->buf); >> - kfree(qp->sq.queue); >> + kvfree(qp->sq.queue); >> return err; >> } >> >> @@ -312,7 +312,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp, >> &qp->rq.queue->ip); >> if (err) { >> vfree(qp->rq.queue->buf); >> - kfree(qp->rq.queue); >> + kvfree(qp->rq.queue); >> return err; >> } >> } >> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c >> index f84ab4469261..e811a655b286 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_queue.c >> +++ b/drivers/infiniband/sw/rxe/rxe_queue.c >> @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, >> if (*num_elem < 0) >> goto err1; >> >> - q = kmalloc(sizeof(*q), GFP_KERNEL); >> + q = kvmalloc(sizeof(*q), GFP_KERNEL); >> if (!q) >> goto err1; >> >> @@ -174,7 +174,7 @@ int rxe_queue_resize(struct rxe_queue *q, >> new_q->buf_size, &new_q->ip); >> if (err) { >> vfree(new_q->buf); >> - kfree(new_q); >> + kvfree(new_q); >> goto err1; >> } >> >> @@ -208,5 +208,5 @@ void rxe_queue_cleanup(struct rxe_queue *q) >> else >> vfree(q->buf); >> >> - kfree(q); >> + kvfree(q); >> } >> diff --git a/drivers/infiniband/sw/rxe/rxe_srq.c b/drivers/infiniband/sw/rxe/rxe_srq.c >> index c41a5fee81f7..c9969d157b87 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_srq.c >> +++ b/drivers/infiniband/sw/rxe/rxe_srq.c >> @@ -132,7 +132,7 @@ int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq, >> q->buf_size, &q->ip); >> if (err) { >> vfree(q->buf); >> - kfree(q); >> + kvfree(q); >> return err; >> } >> >> -- >> 2.17.1 >>
On Sun, Nov 18, 2018 at 09:37:09PM +0800, Yanjun Zhu wrote: > > On 2018/11/18 21:35, Yuval Shaia wrote: > > On Sun, Nov 18, 2018 at 08:21:24AM -0500, Yanjun Zhu wrote: > > > Attempt to allocate physically contiguous memory, but upon > > > failure, fall back to non-contiguous (vmalloc) allocation. > > > > > > Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> > > > --- > > > drivers/infiniband/sw/rxe/rxe_cq.c | 2 +- > > > drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++-- > > > drivers/infiniband/sw/rxe/rxe_queue.c | 6 +++--- > > > drivers/infiniband/sw/rxe/rxe_srq.c | 2 +- > > > 4 files changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c > > > index a57276f2cb84..b85ca024fa58 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_cq.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_cq.c > > > @@ -98,7 +98,7 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe, > > > cq->queue->buf, cq->queue->buf_size, &cq->queue->ip); > > > if (err) { > > > vfree(cq->queue->buf); > > > - kfree(cq->queue); > > > + kvfree(cq->queue); > > But queue is created with kmalloc, what am i missing here? > > Now I use kvmalloc to create queue. > > Zhu Yanjun I see, my bad, i was misled by commit message. Anyway, I do not see here an attempt to allocated with kmalloc and switch to kvmalloc on failure, i see that you just decided to use kvmalloc *instead* of kmalloc, right? > > > > > > Is this patch based on some other patch that is not yet in for-next? > > > > > return err; > > > } > > > diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c > > > index b9710907dac2..fd233f93ac1d 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_qp.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_qp.c > > > @@ -259,7 +259,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, > > > if (err) { > > > vfree(qp->sq.queue->buf); > > > - kfree(qp->sq.queue); > > > + kvfree(qp->sq.queue); > > > return err; > > > } > > > @@ -312,7 +312,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp, > > > &qp->rq.queue->ip); > > > if (err) { > > > vfree(qp->rq.queue->buf); > > > - kfree(qp->rq.queue); > > > + kvfree(qp->rq.queue); > > > return err; > > > } > > > } > > > diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c > > > index f84ab4469261..e811a655b286 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_queue.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_queue.c > > > @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, > > > if (*num_elem < 0) > > > goto err1; > > > - q = kmalloc(sizeof(*q), GFP_KERNEL); > > > + q = kvmalloc(sizeof(*q), GFP_KERNEL); Where is the "fall back" logic? > > > if (!q) > > > goto err1; > > > @@ -174,7 +174,7 @@ int rxe_queue_resize(struct rxe_queue *q, > > > new_q->buf_size, &new_q->ip); > > > if (err) { > > > vfree(new_q->buf); > > > - kfree(new_q); > > > + kvfree(new_q); > > > goto err1; > > > } > > > @@ -208,5 +208,5 @@ void rxe_queue_cleanup(struct rxe_queue *q) > > > else > > > vfree(q->buf); > > > - kfree(q); > > > + kvfree(q); > > > } > > > diff --git a/drivers/infiniband/sw/rxe/rxe_srq.c b/drivers/infiniband/sw/rxe/rxe_srq.c > > > index c41a5fee81f7..c9969d157b87 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_srq.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_srq.c > > > @@ -132,7 +132,7 @@ int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq, > > > q->buf_size, &q->ip); > > > if (err) { > > > vfree(q->buf); > > > - kfree(q); > > > + kvfree(q); > > > return err; > > > } > > > -- > > > 2.17.1 > > >
On 2018/11/19 3:34, Yuval Shaia wrote: > On Sun, Nov 18, 2018 at 09:37:09PM +0800, Yanjun Zhu wrote: >> On 2018/11/18 21:35, Yuval Shaia wrote: >>> On Sun, Nov 18, 2018 at 08:21:24AM -0500, Yanjun Zhu wrote: >>>> Attempt to allocate physically contiguous memory, but upon >>>> failure, fall back to non-contiguous (vmalloc) allocation. >>>> >>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> >>>> --- >>>> drivers/infiniband/sw/rxe/rxe_cq.c | 2 +- >>>> drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++-- >>>> drivers/infiniband/sw/rxe/rxe_queue.c | 6 +++--- >>>> drivers/infiniband/sw/rxe/rxe_srq.c | 2 +- >>>> 4 files changed, 7 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c >>>> index a57276f2cb84..b85ca024fa58 100644 >>>> --- a/drivers/infiniband/sw/rxe/rxe_cq.c >>>> +++ b/drivers/infiniband/sw/rxe/rxe_cq.c >>>> @@ -98,7 +98,7 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe, >>>> cq->queue->buf, cq->queue->buf_size, &cq->queue->ip); >>>> if (err) { >>>> vfree(cq->queue->buf); >>>> - kfree(cq->queue); >>>> + kvfree(cq->queue); >>> But queue is created with kmalloc, what am i missing here? >> Now I use kvmalloc to create queue. >> >> Zhu Yanjun > I see, my bad, i was misled by commit message. > > Anyway, I do not see here an attempt to allocated with kmalloc and switch > to kvmalloc on failure, i see that you just decided to use kvmalloc > *instead* of kmalloc, right? Yes. kvmalloc is to allocate physically contiguous memory. If failure, fall back to non-contiguous (vmalloc) allocation. Zhu Yanjun > >> >>> Is this patch based on some other patch that is not yet in for-next? >>> >>>> return err; >>>> } >>>> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c >>>> index b9710907dac2..fd233f93ac1d 100644 >>>> --- a/drivers/infiniband/sw/rxe/rxe_qp.c >>>> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c >>>> @@ -259,7 +259,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, >>>> if (err) { >>>> vfree(qp->sq.queue->buf); >>>> - kfree(qp->sq.queue); >>>> + kvfree(qp->sq.queue); >>>> return err; >>>> } >>>> @@ -312,7 +312,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp, >>>> &qp->rq.queue->ip); >>>> if (err) { >>>> vfree(qp->rq.queue->buf); >>>> - kfree(qp->rq.queue); >>>> + kvfree(qp->rq.queue); >>>> return err; >>>> } >>>> } >>>> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c >>>> index f84ab4469261..e811a655b286 100644 >>>> --- a/drivers/infiniband/sw/rxe/rxe_queue.c >>>> +++ b/drivers/infiniband/sw/rxe/rxe_queue.c >>>> @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, >>>> if (*num_elem < 0) >>>> goto err1; >>>> - q = kmalloc(sizeof(*q), GFP_KERNEL); >>>> + q = kvmalloc(sizeof(*q), GFP_KERNEL); > Where is the "fall back" logic? > >>>> if (!q) >>>> goto err1; >>>> @@ -174,7 +174,7 @@ int rxe_queue_resize(struct rxe_queue *q, >>>> new_q->buf_size, &new_q->ip); >>>> if (err) { >>>> vfree(new_q->buf); >>>> - kfree(new_q); >>>> + kvfree(new_q); >>>> goto err1; >>>> } >>>> @@ -208,5 +208,5 @@ void rxe_queue_cleanup(struct rxe_queue *q) >>>> else >>>> vfree(q->buf); >>>> - kfree(q); >>>> + kvfree(q); >>>> } >>>> diff --git a/drivers/infiniband/sw/rxe/rxe_srq.c b/drivers/infiniband/sw/rxe/rxe_srq.c >>>> index c41a5fee81f7..c9969d157b87 100644 >>>> --- a/drivers/infiniband/sw/rxe/rxe_srq.c >>>> +++ b/drivers/infiniband/sw/rxe/rxe_srq.c >>>> @@ -132,7 +132,7 @@ int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq, >>>> q->buf_size, &q->ip); >>>> if (err) { >>>> vfree(q->buf); >>>> - kfree(q); >>>> + kvfree(q); >>>> return err; >>>> } >>>> -- >>>> 2.17.1 >>>>
On Sun, Nov 18, 2018 at 08:21:24AM -0500, Zhu Yanjun wrote: > diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c > index f84ab4469261..e811a655b286 100644 > +++ b/drivers/infiniband/sw/rxe/rxe_queue.c > @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, > if (*num_elem < 0) > goto err1; > > - q = kmalloc(sizeof(*q), GFP_KERNEL); > + q = kvmalloc(sizeof(*q), GFP_KERNEL); sizeof(*q) looks like it is about 64 bytes It makes no sense to use kvmalloc unless the allocation size is > PAGE_SIZE Jason
On 2018/11/30 8:09, Jason Gunthorpe wrote: > On Sun, Nov 18, 2018 at 08:21:24AM -0500, Zhu Yanjun wrote: > >> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c >> index f84ab4469261..e811a655b286 100644 >> +++ b/drivers/infiniband/sw/rxe/rxe_queue.c >> @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, >> if (*num_elem < 0) >> goto err1; >> >> - q = kmalloc(sizeof(*q), GFP_KERNEL); >> + q = kvmalloc(sizeof(*q), GFP_KERNEL); > sizeof(*q) looks like it is about 64 bytes > > It makes no sense to use kvmalloc unless the allocation size is > > PAGE_SIZE Sure. Thanks /* * It doesn't really make sense to fallback to vmalloc for sub page * requests */ if (ret || size <= PAGE_SIZE) return ret; Zhu Yanjun > > Jason
diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c index a57276f2cb84..b85ca024fa58 100644 --- a/drivers/infiniband/sw/rxe/rxe_cq.c +++ b/drivers/infiniband/sw/rxe/rxe_cq.c @@ -98,7 +98,7 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe, cq->queue->buf, cq->queue->buf_size, &cq->queue->ip); if (err) { vfree(cq->queue->buf); - kfree(cq->queue); + kvfree(cq->queue); return err; } diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c index b9710907dac2..fd233f93ac1d 100644 --- a/drivers/infiniband/sw/rxe/rxe_qp.c +++ b/drivers/infiniband/sw/rxe/rxe_qp.c @@ -259,7 +259,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, if (err) { vfree(qp->sq.queue->buf); - kfree(qp->sq.queue); + kvfree(qp->sq.queue); return err; } @@ -312,7 +312,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp, &qp->rq.queue->ip); if (err) { vfree(qp->rq.queue->buf); - kfree(qp->rq.queue); + kvfree(qp->rq.queue); return err; } } diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c index f84ab4469261..e811a655b286 100644 --- a/drivers/infiniband/sw/rxe/rxe_queue.c +++ b/drivers/infiniband/sw/rxe/rxe_queue.c @@ -91,7 +91,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, if (*num_elem < 0) goto err1; - q = kmalloc(sizeof(*q), GFP_KERNEL); + q = kvmalloc(sizeof(*q), GFP_KERNEL); if (!q) goto err1; @@ -174,7 +174,7 @@ int rxe_queue_resize(struct rxe_queue *q, new_q->buf_size, &new_q->ip); if (err) { vfree(new_q->buf); - kfree(new_q); + kvfree(new_q); goto err1; } @@ -208,5 +208,5 @@ void rxe_queue_cleanup(struct rxe_queue *q) else vfree(q->buf); - kfree(q); + kvfree(q); } diff --git a/drivers/infiniband/sw/rxe/rxe_srq.c b/drivers/infiniband/sw/rxe/rxe_srq.c index c41a5fee81f7..c9969d157b87 100644 --- a/drivers/infiniband/sw/rxe/rxe_srq.c +++ b/drivers/infiniband/sw/rxe/rxe_srq.c @@ -132,7 +132,7 @@ int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq, q->buf_size, &q->ip); if (err) { vfree(q->buf); - kfree(q); + kvfree(q); return err; }
Attempt to allocate physically contiguous memory, but upon failure, fall back to non-contiguous (vmalloc) allocation. Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> --- drivers/infiniband/sw/rxe/rxe_cq.c | 2 +- drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++-- drivers/infiniband/sw/rxe/rxe_queue.c | 6 +++--- drivers/infiniband/sw/rxe/rxe_srq.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-)