* [PATCH] loop: use BIO list management functions @ 2009-04-17 6:32 Akinobu Mita 2009-04-17 6:37 ` Christoph Hellwig 2009-04-17 6:40 ` Jens Axboe 0 siblings, 2 replies; 7+ messages in thread From: Akinobu Mita @ 2009-04-17 6:32 UTC (permalink / raw) To: linux-kernel; +Cc: Jens Axboe, Christoph Hellwig Impact: cleanup Cc: Jens Axboe <axboe@kernel.dk> Cc: Christoph Hellwig <hch@infradead.org> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/block/loop.c | 26 +++++++------------------- include/linux/bio.h | 2 +- include/linux/loop.h | 3 +-- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index ddae808..9ca4bb0 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -511,11 +511,7 @@ out: */ static void loop_add_bio(struct loop_device *lo, struct bio *bio) { - if (lo->lo_biotail) { - lo->lo_biotail->bi_next = bio; - lo->lo_biotail = bio; - } else - lo->lo_bio = lo->lo_biotail = bio; + bio_list_add(&lo->lo_bio_list, bio); } /* @@ -523,16 +519,7 @@ static void loop_add_bio(struct loop_device *lo, struct bio *bio) */ static struct bio *loop_get_bio(struct loop_device *lo) { - struct bio *bio; - - if ((bio = lo->lo_bio)) { - if (bio == lo->lo_biotail) - lo->lo_biotail = NULL; - lo->lo_bio = bio->bi_next; - bio->bi_next = NULL; - } - - return bio; + return bio_list_pop(&lo->lo_bio_list); } static int loop_make_request(struct request_queue *q, struct bio *old_bio) @@ -609,12 +596,13 @@ static int loop_thread(void *data) set_user_nice(current, -20); - while (!kthread_should_stop() || lo->lo_bio) { + while (!kthread_should_stop() || !bio_list_empty(&lo->lo_bio_list)) { wait_event_interruptible(lo->lo_event, - lo->lo_bio || kthread_should_stop()); + !bio_list_empty(&lo->lo_bio_list) || + kthread_should_stop()); - if (!lo->lo_bio) + if (bio_list_empty(&lo->lo_bio_list)) continue; spin_lock_irq(&lo->lo_lock); bio = loop_get_bio(lo); @@ -841,7 +829,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, lo->old_gfp_mask = mapping_gfp_mask(mapping); mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS)); - lo->lo_bio = lo->lo_biotail = NULL; + bio_list_init(&lo->lo_bio_list); /* * set queue make_request_fn, and add limits based on lower level diff --git a/include/linux/bio.h b/include/linux/bio.h index b89cf2d..631153b 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -505,7 +505,7 @@ static inline int bio_has_data(struct bio *bio) } /* - * BIO list managment for use by remapping drivers (e.g. DM or MD). + * BIO list management for use by remapping drivers (e.g. DM or MD) and loop. * * A bio_list anchors a singly-linked list of bios chained through the bi_next * member of the bio. The bio_list also caches the last list member to allow diff --git a/include/linux/loop.h b/include/linux/loop.h index 4072544..66c194e 100644 --- a/include/linux/loop.h +++ b/include/linux/loop.h @@ -56,8 +56,7 @@ struct loop_device { gfp_t old_gfp_mask; spinlock_t lo_lock; - struct bio *lo_bio; - struct bio *lo_biotail; + struct bio_list lo_bio_list; int lo_state; struct mutex lo_ctl_mutex; struct task_struct *lo_thread; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] loop: use BIO list management functions 2009-04-17 6:32 [PATCH] loop: use BIO list management functions Akinobu Mita @ 2009-04-17 6:37 ` Christoph Hellwig 2009-04-17 7:14 ` Jens Axboe 2009-04-17 7:32 ` Akinobu Mita 2009-04-17 6:40 ` Jens Axboe 1 sibling, 2 replies; 7+ messages in thread From: Christoph Hellwig @ 2009-04-17 6:37 UTC (permalink / raw) To: Akinobu Mita; +Cc: linux-kernel, Jens Axboe, Christoph Hellwig On Fri, Apr 17, 2009 at 03:32:17PM +0900, Akinobu Mita wrote: > Impact: cleanup I though we had pretty broad agreement that these lines are utter crap? :) > static void loop_add_bio(struct loop_device *lo, struct bio *bio) > { > - if (lo->lo_biotail) { > - lo->lo_biotail->bi_next = bio; > - lo->lo_biotail = bio; > - } else > - lo->lo_bio = lo->lo_biotail = bio; > + bio_list_add(&lo->lo_bio_list, bio); > } This function only has one caller, no need to keep it. > static struct bio *loop_get_bio(struct loop_device *lo) > { > - struct bio *bio; > - > - if ((bio = lo->lo_bio)) { > - if (bio == lo->lo_biotail) > - lo->lo_biotail = NULL; > - lo->lo_bio = bio->bi_next; > - bio->bi_next = NULL; > - } > - > - return bio; > + return bio_list_pop(&lo->lo_bio_list); > } Same here. > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -505,7 +505,7 @@ static inline int bio_has_data(struct bio *bio) > } > > /* > - * BIO list managment for use by remapping drivers (e.g. DM or MD). > + * BIO list management for use by remapping drivers (e.g. DM or MD) and loop. > * I don't think this needs changing, that was just examples. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] loop: use BIO list management functions 2009-04-17 6:37 ` Christoph Hellwig @ 2009-04-17 7:14 ` Jens Axboe 2009-04-17 7:32 ` Akinobu Mita 1 sibling, 0 replies; 7+ messages in thread From: Jens Axboe @ 2009-04-17 7:14 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Akinobu Mita, linux-kernel On Fri, Apr 17 2009, Christoph Hellwig wrote: > On Fri, Apr 17, 2009 at 03:32:17PM +0900, Akinobu Mita wrote: > > Impact: cleanup > > I though we had pretty broad agreement that these lines are utter crap? > :) Agree on that... -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] loop: use BIO list management functions 2009-04-17 6:37 ` Christoph Hellwig 2009-04-17 7:14 ` Jens Axboe @ 2009-04-17 7:32 ` Akinobu Mita 1 sibling, 0 replies; 7+ messages in thread From: Akinobu Mita @ 2009-04-17 7:32 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-kernel, Jens Axboe On Fri, Apr 17, 2009 at 02:37:02AM -0400, Christoph Hellwig wrote: > > /* > > - * BIO list managment for use by remapping drivers (e.g. DM or MD). > > + * BIO list management for use by remapping drivers (e.g. DM or MD) and loop. > > * > > > I don't think this needs changing, that was just examples. OK. I kept only s/managment/management/ typo in new patch. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] loop: use BIO list management functions 2009-04-17 6:32 [PATCH] loop: use BIO list management functions Akinobu Mita 2009-04-17 6:37 ` Christoph Hellwig @ 2009-04-17 6:40 ` Jens Axboe 2009-04-17 7:30 ` Akinobu Mita 1 sibling, 1 reply; 7+ messages in thread From: Jens Axboe @ 2009-04-17 6:40 UTC (permalink / raw) To: Akinobu Mita; +Cc: linux-kernel, Christoph Hellwig On Fri, Apr 17 2009, Akinobu Mita wrote: > Impact: cleanup This looks good, but please provide a better description than just a silly impact notice. I've filled it in for you for this one, but in the future the description should say what the patch does. It's queued for 2.6.31. > > Cc: Jens Axboe <axboe@kernel.dk> > Cc: Christoph Hellwig <hch@infradead.org> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > drivers/block/loop.c | 26 +++++++------------------- > include/linux/bio.h | 2 +- > include/linux/loop.h | 3 +-- > 3 files changed, 9 insertions(+), 22 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index ddae808..9ca4bb0 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -511,11 +511,7 @@ out: > */ > static void loop_add_bio(struct loop_device *lo, struct bio *bio) > { > - if (lo->lo_biotail) { > - lo->lo_biotail->bi_next = bio; > - lo->lo_biotail = bio; > - } else > - lo->lo_bio = lo->lo_biotail = bio; > + bio_list_add(&lo->lo_bio_list, bio); > } > > /* > @@ -523,16 +519,7 @@ static void loop_add_bio(struct loop_device *lo, struct bio *bio) > */ > static struct bio *loop_get_bio(struct loop_device *lo) > { > - struct bio *bio; > - > - if ((bio = lo->lo_bio)) { > - if (bio == lo->lo_biotail) > - lo->lo_biotail = NULL; > - lo->lo_bio = bio->bi_next; > - bio->bi_next = NULL; > - } > - > - return bio; > + return bio_list_pop(&lo->lo_bio_list); > } > > static int loop_make_request(struct request_queue *q, struct bio *old_bio) > @@ -609,12 +596,13 @@ static int loop_thread(void *data) > > set_user_nice(current, -20); > > - while (!kthread_should_stop() || lo->lo_bio) { > + while (!kthread_should_stop() || !bio_list_empty(&lo->lo_bio_list)) { > > wait_event_interruptible(lo->lo_event, > - lo->lo_bio || kthread_should_stop()); > + !bio_list_empty(&lo->lo_bio_list) || > + kthread_should_stop()); > > - if (!lo->lo_bio) > + if (bio_list_empty(&lo->lo_bio_list)) > continue; > spin_lock_irq(&lo->lo_lock); > bio = loop_get_bio(lo); > @@ -841,7 +829,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, > lo->old_gfp_mask = mapping_gfp_mask(mapping); > mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS)); > > - lo->lo_bio = lo->lo_biotail = NULL; > + bio_list_init(&lo->lo_bio_list); > > /* > * set queue make_request_fn, and add limits based on lower level > diff --git a/include/linux/bio.h b/include/linux/bio.h > index b89cf2d..631153b 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -505,7 +505,7 @@ static inline int bio_has_data(struct bio *bio) > } > > /* > - * BIO list managment for use by remapping drivers (e.g. DM or MD). > + * BIO list management for use by remapping drivers (e.g. DM or MD) and loop. > * > * A bio_list anchors a singly-linked list of bios chained through the bi_next > * member of the bio. The bio_list also caches the last list member to allow > diff --git a/include/linux/loop.h b/include/linux/loop.h > index 4072544..66c194e 100644 > --- a/include/linux/loop.h > +++ b/include/linux/loop.h > @@ -56,8 +56,7 @@ struct loop_device { > gfp_t old_gfp_mask; > > spinlock_t lo_lock; > - struct bio *lo_bio; > - struct bio *lo_biotail; > + struct bio_list lo_bio_list; > int lo_state; > struct mutex lo_ctl_mutex; > struct task_struct *lo_thread; -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] loop: use BIO list management functions 2009-04-17 6:40 ` Jens Axboe @ 2009-04-17 7:30 ` Akinobu Mita 2009-04-17 7:33 ` Jens Axboe 0 siblings, 1 reply; 7+ messages in thread From: Akinobu Mita @ 2009-04-17 7:30 UTC (permalink / raw) To: Jens Axboe; +Cc: linux-kernel, Christoph Hellwig On Fri, Apr 17, 2009 at 08:40:35AM +0200, Jens Axboe wrote: > On Fri, Apr 17 2009, Akinobu Mita wrote: > > Impact: cleanup > > This looks good, but please provide a better description than just a > silly impact notice. I've filled it in for you for this one, but in the > future the description should say what the patch does. > > It's queued for 2.6.31. OK, I added the description and Christoph's feedbacks. Subject: loop: use BIO list management functions There are BIO list management helpers in bio.h. This uses them to reduce code duplication in loopback driver. Cc: Jens Axboe <axboe@kernel.dk> Cc: Christoph Hellwig <hch@infradead.org> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/block/loop.c | 42 +++++++----------------------------------- include/linux/bio.h | 2 +- include/linux/loop.h | 3 +-- 3 files changed, 9 insertions(+), 38 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index ddae808..2b70545 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -506,35 +506,6 @@ out: return ret; } -/* - * Add bio to back of pending list - */ -static void loop_add_bio(struct loop_device *lo, struct bio *bio) -{ - if (lo->lo_biotail) { - lo->lo_biotail->bi_next = bio; - lo->lo_biotail = bio; - } else - lo->lo_bio = lo->lo_biotail = bio; -} - -/* - * Grab first pending buffer - */ -static struct bio *loop_get_bio(struct loop_device *lo) -{ - struct bio *bio; - - if ((bio = lo->lo_bio)) { - if (bio == lo->lo_biotail) - lo->lo_biotail = NULL; - lo->lo_bio = bio->bi_next; - bio->bi_next = NULL; - } - - return bio; -} - static int loop_make_request(struct request_queue *q, struct bio *old_bio) { struct loop_device *lo = q->queuedata; @@ -550,7 +521,7 @@ static int loop_make_request(struct request_queue *q, struct bio *old_bio) goto out; if (unlikely(rw == WRITE && (lo->lo_flags & LO_FLAGS_READ_ONLY))) goto out; - loop_add_bio(lo, old_bio); + bio_list_add(&lo->lo_bio_list, old_bio); wake_up(&lo->lo_event); spin_unlock_irq(&lo->lo_lock); return 0; @@ -609,15 +580,16 @@ static int loop_thread(void *data) set_user_nice(current, -20); - while (!kthread_should_stop() || lo->lo_bio) { + while (!kthread_should_stop() || !bio_list_empty(&lo->lo_bio_list)) { wait_event_interruptible(lo->lo_event, - lo->lo_bio || kthread_should_stop()); + !bio_list_empty(&lo->lo_bio_list) || + kthread_should_stop()); - if (!lo->lo_bio) + if (bio_list_empty(&lo->lo_bio_list)) continue; spin_lock_irq(&lo->lo_lock); - bio = loop_get_bio(lo); + bio = bio_list_pop(&lo->lo_bio_list); spin_unlock_irq(&lo->lo_lock); BUG_ON(!bio); @@ -841,7 +813,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, lo->old_gfp_mask = mapping_gfp_mask(mapping); mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS)); - lo->lo_bio = lo->lo_biotail = NULL; + bio_list_init(&lo->lo_bio_list); /* * set queue make_request_fn, and add limits based on lower level diff --git a/include/linux/bio.h b/include/linux/bio.h index b89cf2d..9388c55 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -505,7 +505,7 @@ static inline int bio_has_data(struct bio *bio) } /* - * BIO list managment for use by remapping drivers (e.g. DM or MD). + * BIO list management for use by remapping drivers (e.g. DM or MD). * * A bio_list anchors a singly-linked list of bios chained through the bi_next * member of the bio. The bio_list also caches the last list member to allow diff --git a/include/linux/loop.h b/include/linux/loop.h index 4072544..66c194e 100644 --- a/include/linux/loop.h +++ b/include/linux/loop.h @@ -56,8 +56,7 @@ struct loop_device { gfp_t old_gfp_mask; spinlock_t lo_lock; - struct bio *lo_bio; - struct bio *lo_biotail; + struct bio_list lo_bio_list; int lo_state; struct mutex lo_ctl_mutex; struct task_struct *lo_thread; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] loop: use BIO list management functions 2009-04-17 7:30 ` Akinobu Mita @ 2009-04-17 7:33 ` Jens Axboe 0 siblings, 0 replies; 7+ messages in thread From: Jens Axboe @ 2009-04-17 7:33 UTC (permalink / raw) To: Akinobu Mita; +Cc: linux-kernel, Christoph Hellwig On Fri, Apr 17 2009, Akinobu Mita wrote: > On Fri, Apr 17, 2009 at 08:40:35AM +0200, Jens Axboe wrote: > > On Fri, Apr 17 2009, Akinobu Mita wrote: > > > Impact: cleanup > > > > This looks good, but please provide a better description than just a > > silly impact notice. I've filled it in for you for this one, but in the > > future the description should say what the patch does. > > > > It's queued for 2.6.31. > > OK, I added the description and Christoph's feedbacks. Technically, you should not include the bio.h change with this patch, as it has nothing to do with loop at all. > > Subject: loop: use BIO list management functions > > There are BIO list management helpers in bio.h. > This uses them to reduce code duplication in loopback driver. > > Cc: Jens Axboe <axboe@kernel.dk> > Cc: Christoph Hellwig <hch@infradead.org> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > drivers/block/loop.c | 42 +++++++----------------------------------- > include/linux/bio.h | 2 +- > include/linux/loop.h | 3 +-- > 3 files changed, 9 insertions(+), 38 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index ddae808..2b70545 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -506,35 +506,6 @@ out: > return ret; > } > > -/* > - * Add bio to back of pending list > - */ > -static void loop_add_bio(struct loop_device *lo, struct bio *bio) > -{ > - if (lo->lo_biotail) { > - lo->lo_biotail->bi_next = bio; > - lo->lo_biotail = bio; > - } else > - lo->lo_bio = lo->lo_biotail = bio; > -} > - > -/* > - * Grab first pending buffer > - */ > -static struct bio *loop_get_bio(struct loop_device *lo) > -{ > - struct bio *bio; > - > - if ((bio = lo->lo_bio)) { > - if (bio == lo->lo_biotail) > - lo->lo_biotail = NULL; > - lo->lo_bio = bio->bi_next; > - bio->bi_next = NULL; > - } > - > - return bio; > -} > - > static int loop_make_request(struct request_queue *q, struct bio *old_bio) > { > struct loop_device *lo = q->queuedata; > @@ -550,7 +521,7 @@ static int loop_make_request(struct request_queue *q, struct bio *old_bio) > goto out; > if (unlikely(rw == WRITE && (lo->lo_flags & LO_FLAGS_READ_ONLY))) > goto out; > - loop_add_bio(lo, old_bio); > + bio_list_add(&lo->lo_bio_list, old_bio); > wake_up(&lo->lo_event); > spin_unlock_irq(&lo->lo_lock); > return 0; > @@ -609,15 +580,16 @@ static int loop_thread(void *data) > > set_user_nice(current, -20); > > - while (!kthread_should_stop() || lo->lo_bio) { > + while (!kthread_should_stop() || !bio_list_empty(&lo->lo_bio_list)) { > > wait_event_interruptible(lo->lo_event, > - lo->lo_bio || kthread_should_stop()); > + !bio_list_empty(&lo->lo_bio_list) || > + kthread_should_stop()); > > - if (!lo->lo_bio) > + if (bio_list_empty(&lo->lo_bio_list)) > continue; > spin_lock_irq(&lo->lo_lock); > - bio = loop_get_bio(lo); > + bio = bio_list_pop(&lo->lo_bio_list); > spin_unlock_irq(&lo->lo_lock); > > BUG_ON(!bio); > @@ -841,7 +813,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, > lo->old_gfp_mask = mapping_gfp_mask(mapping); > mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS)); > > - lo->lo_bio = lo->lo_biotail = NULL; > + bio_list_init(&lo->lo_bio_list); > > /* > * set queue make_request_fn, and add limits based on lower level > diff --git a/include/linux/bio.h b/include/linux/bio.h > index b89cf2d..9388c55 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -505,7 +505,7 @@ static inline int bio_has_data(struct bio *bio) > } > > /* > - * BIO list managment for use by remapping drivers (e.g. DM or MD). > + * BIO list management for use by remapping drivers (e.g. DM or MD). > * > * A bio_list anchors a singly-linked list of bios chained through the bi_next > * member of the bio. The bio_list also caches the last list member to allow > diff --git a/include/linux/loop.h b/include/linux/loop.h > index 4072544..66c194e 100644 > --- a/include/linux/loop.h > +++ b/include/linux/loop.h > @@ -56,8 +56,7 @@ struct loop_device { > gfp_t old_gfp_mask; > > spinlock_t lo_lock; > - struct bio *lo_bio; > - struct bio *lo_biotail; > + struct bio_list lo_bio_list; > int lo_state; > struct mutex lo_ctl_mutex; > struct task_struct *lo_thread; -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-04-17 7:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-04-17 6:32 [PATCH] loop: use BIO list management functions Akinobu Mita 2009-04-17 6:37 ` Christoph Hellwig 2009-04-17 7:14 ` Jens Axboe 2009-04-17 7:32 ` Akinobu Mita 2009-04-17 6:40 ` Jens Axboe 2009-04-17 7:30 ` Akinobu Mita 2009-04-17 7:33 ` Jens Axboe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox