RE: [PATCH] elevator: fix double release for elevator module
Hi Jens and Jeff, Thanks for your review and help! :) Regards, > -Original Message- > From: Jens Axboe [mailto:ax...@kernel.dk] > Sent: Friday, April 24, 2015 12:49 AM > To: Jeff Moyer; Chao Yu > Cc: linux-kernel@vger.kernel.org > Subject: Re: [PATCH] elevator: fix double release for elevator module > > On 04/23/2015 08:59 AM, Jeff Moyer wrote: > > Chao Yu writes: > > > >> Our issue is descripted in below call path: > >> ->elevator_init > >> ->elevator_init_fn > >>->{cfq,deadline,noop}_init_queue > >> ->elevator_alloc > >> ->kzalloc_node > >> fail to call kzalloc_node and then put module in elevator_alloc; > >> fail to call elevator_init_fn and then put module again in elevator_init. > >> > >> Remove elevator_put invoking in error path of elevator_alloc to avoid > >> double release issue. > >> > >> Signed-off-by: Chao Yu > >> --- > >> block/elevator.c | 5 + > >> 1 file changed, 1 insertion(+), 4 deletions(-) > >> > >> diff --git a/block/elevator.c b/block/elevator.c > >> index d146a5e..8985038 100644 > >> --- a/block/elevator.c > >> +++ b/block/elevator.c > >> @@ -157,7 +157,7 @@ struct elevator_queue *elevator_alloc(struct > >> request_queue *q, > >> > >>eq = kzalloc_node(sizeof(*eq), GFP_KERNEL, q->node); > >>if (unlikely(!eq)) > >> - goto err; > >> + return NULL; > >> > >>eq->type = e; > >>kobject_init(&eq->kobj, &elv_ktype); > >> @@ -165,9 +165,6 @@ struct elevator_queue *elevator_alloc(struct > >> request_queue *q, > >>hash_init(eq->hash); > >> > >>return eq; > >> -err: > >> - elevator_put(e); > >> - return NULL; > >> } > >> EXPORT_SYMBOL(elevator_alloc); > > > > You could have posted the two patches together, as they are related. > > Anyway, looks good to me. > > > > Reviewed-by: Jeff Moyer > > Agree, it should be one patch. I've combined them, and applied the fix > for 4.1. Thanks. > > -- > Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] elevator: fix double release for elevator module
On 04/23/2015 08:59 AM, Jeff Moyer wrote: Chao Yu writes: Our issue is descripted in below call path: ->elevator_init ->elevator_init_fn ->{cfq,deadline,noop}_init_queue ->elevator_alloc ->kzalloc_node fail to call kzalloc_node and then put module in elevator_alloc; fail to call elevator_init_fn and then put module again in elevator_init. Remove elevator_put invoking in error path of elevator_alloc to avoid double release issue. Signed-off-by: Chao Yu --- block/elevator.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/block/elevator.c b/block/elevator.c index d146a5e..8985038 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -157,7 +157,7 @@ struct elevator_queue *elevator_alloc(struct request_queue *q, eq = kzalloc_node(sizeof(*eq), GFP_KERNEL, q->node); if (unlikely(!eq)) - goto err; + return NULL; eq->type = e; kobject_init(&eq->kobj, &elv_ktype); @@ -165,9 +165,6 @@ struct elevator_queue *elevator_alloc(struct request_queue *q, hash_init(eq->hash); return eq; -err: - elevator_put(e); - return NULL; } EXPORT_SYMBOL(elevator_alloc); You could have posted the two patches together, as they are related. Anyway, looks good to me. Reviewed-by: Jeff Moyer Agree, it should be one patch. I've combined them, and applied the fix for 4.1. Thanks. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] elevator: fix double release for elevator module
Chao Yu writes: > Our issue is descripted in below call path: > ->elevator_init > ->elevator_init_fn > ->{cfq,deadline,noop}_init_queue >->elevator_alloc > ->kzalloc_node >fail to call kzalloc_node and then put module in elevator_alloc; > fail to call elevator_init_fn and then put module again in elevator_init. > > Remove elevator_put invoking in error path of elevator_alloc to avoid > double release issue. > > Signed-off-by: Chao Yu > --- > block/elevator.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/block/elevator.c b/block/elevator.c > index d146a5e..8985038 100644 > --- a/block/elevator.c > +++ b/block/elevator.c > @@ -157,7 +157,7 @@ struct elevator_queue *elevator_alloc(struct > request_queue *q, > > eq = kzalloc_node(sizeof(*eq), GFP_KERNEL, q->node); > if (unlikely(!eq)) > - goto err; > + return NULL; > > eq->type = e; > kobject_init(&eq->kobj, &elv_ktype); > @@ -165,9 +165,6 @@ struct elevator_queue *elevator_alloc(struct > request_queue *q, > hash_init(eq->hash); > > return eq; > -err: > - elevator_put(e); > - return NULL; > } > EXPORT_SYMBOL(elevator_alloc); You could have posted the two patches together, as they are related. Anyway, looks good to me. Reviewed-by: Jeff Moyer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/