FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 08-06-2012, 10:08 PM
Kent Overstreet
 
Default pktcdvd: Switch to bio_kmalloc()

This is prep work for killing bi_destructor - previously, pktcdvd had
its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
necessitating its own bi_destructor implementation.

v5: Un-reorder some functions, to make the patch easier to review

Signed-off-by: Kent Overstreet <koverstreet@google.com>
---
drivers/block/pktcdvd.c | 67 +++++++++++-----------------------------------
1 files changed, 16 insertions(+), 51 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index ba66e44..ae55f08 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -101,6 +101,8 @@ static struct dentry *pkt_debugfs_root = NULL; /* /sys/kernel/debug/pktcdvd */
static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev);
static int pkt_remove_dev(dev_t pkt_dev);
static int pkt_seq_show(struct seq_file *m, void *p);
+static void pkt_end_io_read(struct bio *bio, int err);
+static void pkt_end_io_packet_write(struct bio *bio, int err);



@@ -522,38 +524,6 @@ static void pkt_bio_finished(struct pktcdvd_device *pd)
}
}

-static void pkt_bio_destructor(struct bio *bio)
-{
- kfree(bio->bi_io_vec);
- kfree(bio);
-}
-
-static struct bio *pkt_bio_alloc(int nr_iovecs)
-{
- struct bio_vec *bvl = NULL;
- struct bio *bio;
-
- bio = kmalloc(sizeof(struct bio), GFP_KERNEL);
- if (!bio)
- goto no_bio;
- bio_init(bio);
-
- bvl = kcalloc(nr_iovecs, sizeof(struct bio_vec), GFP_KERNEL);
- if (!bvl)
- goto no_bvl;
-
- bio->bi_max_vecs = nr_iovecs;
- bio->bi_io_vec = bvl;
- bio->bi_destructor = pkt_bio_destructor;
-
- return bio;
-
- no_bvl:
- kfree(bio);
- no_bio:
- return NULL;
-}
-
/*
* Allocate a packet_data struct
*/
@@ -567,10 +537,13 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
goto no_pkt;

pkt->frames = frames;
- pkt->w_bio = pkt_bio_alloc(frames);
+ pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames);
if (!pkt->w_bio)
goto no_bio;

+ pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
+ pkt->w_bio->bi_private = pkt;
+
for (i = 0; i < frames / FRAMES_PER_PAGE; i++) {
pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO);
if (!pkt->pages[i])
@@ -581,9 +554,12 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
bio_list_init(&pkt->orig_bios);

for (i = 0; i < frames; i++) {
- struct bio *bio = pkt_bio_alloc(1);
+ struct bio *bio = bio_kmalloc(GFP_KERNEL, 1);
if (!bio)
goto no_rd_bio;
+
+ bio->bi_end_io = pkt_end_io_read;
+ bio->bi_private = pkt;
pkt->r_bios[i] = bio;
}

@@ -1111,21 +1087,15 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
* Schedule reads for missing parts of the packet.
*/
for (f = 0; f < pkt->frames; f++) {
- struct bio_vec *vec;
-
int p, offset;
+
if (written[f])
continue;
+
bio = pkt->r_bios[f];
- vec = bio->bi_io_vec;
- bio_init(bio);
- bio->bi_max_vecs = 1;
- bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
- bio->bi_bdev = pd->bdev;
- bio->bi_end_io = pkt_end_io_read;
- bio->bi_private = pkt;
- bio->bi_io_vec = vec;
- bio->bi_destructor = pkt_bio_destructor;
+ bio_reset(bio);
+ bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
+ bio->bi_bdev = pd->bdev;

p = (f * CD_FRAMESIZE) / PAGE_SIZE;
offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
@@ -1418,14 +1388,9 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
}

/* Start the write request */
- bio_init(pkt->w_bio);
- pkt->w_bio->bi_max_vecs = PACKET_MAX_SIZE;
+ bio_reset(pkt->w_bio);
pkt->w_bio->bi_sector = pkt->sector;
pkt->w_bio->bi_bdev = pd->bdev;
- pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
- pkt->w_bio->bi_private = pkt;
- pkt->w_bio->bi_io_vec = bvec;
- pkt->w_bio->bi_destructor = pkt_bio_destructor;
for (f = 0; f < pkt->frames; f++)
if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset))
BUG();
--
1.7.7.3

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-08-2012, 10:13 PM
Tejun Heo
 
Default pktcdvd: Switch to bio_kmalloc()

Hello,

On Mon, Aug 06, 2012 at 03:08:33PM -0700, Kent Overstreet wrote:
> This is prep work for killing bi_destructor - previously, pktcdvd had
> its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
> necessitating its own bi_destructor implementation.
>
> v5: Un-reorder some functions, to make the patch easier to review
>
> Signed-off-by: Kent Overstreet <koverstreet@google.com>

Please Cc: the maintainers. Cc'ing Peter Osterlund and keeping the
whole body for him.

Generally looks good to me. How is this tested?

Thanks.

> ---
> drivers/block/pktcdvd.c | 67 +++++++++++-----------------------------------
> 1 files changed, 16 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index ba66e44..ae55f08 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -101,6 +101,8 @@ static struct dentry *pkt_debugfs_root = NULL; /* /sys/kernel/debug/pktcdvd */
> static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev);
> static int pkt_remove_dev(dev_t pkt_dev);
> static int pkt_seq_show(struct seq_file *m, void *p);
> +static void pkt_end_io_read(struct bio *bio, int err);
> +static void pkt_end_io_packet_write(struct bio *bio, int err);
>
>
>
> @@ -522,38 +524,6 @@ static void pkt_bio_finished(struct pktcdvd_device *pd)
> }
> }
>
> -static void pkt_bio_destructor(struct bio *bio)
> -{
> - kfree(bio->bi_io_vec);
> - kfree(bio);
> -}
> -
> -static struct bio *pkt_bio_alloc(int nr_iovecs)
> -{
> - struct bio_vec *bvl = NULL;
> - struct bio *bio;
> -
> - bio = kmalloc(sizeof(struct bio), GFP_KERNEL);
> - if (!bio)
> - goto no_bio;
> - bio_init(bio);
> -
> - bvl = kcalloc(nr_iovecs, sizeof(struct bio_vec), GFP_KERNEL);
> - if (!bvl)
> - goto no_bvl;
> -
> - bio->bi_max_vecs = nr_iovecs;
> - bio->bi_io_vec = bvl;
> - bio->bi_destructor = pkt_bio_destructor;
> -
> - return bio;
> -
> - no_bvl:
> - kfree(bio);
> - no_bio:
> - return NULL;
> -}
> -
> /*
> * Allocate a packet_data struct
> */
> @@ -567,10 +537,13 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
> goto no_pkt;
>
> pkt->frames = frames;
> - pkt->w_bio = pkt_bio_alloc(frames);
> + pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames);
> if (!pkt->w_bio)
> goto no_bio;
>
> + pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
> + pkt->w_bio->bi_private = pkt;
> +
> for (i = 0; i < frames / FRAMES_PER_PAGE; i++) {
> pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO);
> if (!pkt->pages[i])
> @@ -581,9 +554,12 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
> bio_list_init(&pkt->orig_bios);
>
> for (i = 0; i < frames; i++) {
> - struct bio *bio = pkt_bio_alloc(1);
> + struct bio *bio = bio_kmalloc(GFP_KERNEL, 1);
> if (!bio)
> goto no_rd_bio;
> +
> + bio->bi_end_io = pkt_end_io_read;
> + bio->bi_private = pkt;
> pkt->r_bios[i] = bio;
> }
>
> @@ -1111,21 +1087,15 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
> * Schedule reads for missing parts of the packet.
> */
> for (f = 0; f < pkt->frames; f++) {
> - struct bio_vec *vec;
> -
> int p, offset;
> +
> if (written[f])
> continue;
> +
> bio = pkt->r_bios[f];
> - vec = bio->bi_io_vec;
> - bio_init(bio);
> - bio->bi_max_vecs = 1;
> - bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
> - bio->bi_bdev = pd->bdev;
> - bio->bi_end_io = pkt_end_io_read;
> - bio->bi_private = pkt;
> - bio->bi_io_vec = vec;
> - bio->bi_destructor = pkt_bio_destructor;
> + bio_reset(bio);
> + bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
> + bio->bi_bdev = pd->bdev;
>
> p = (f * CD_FRAMESIZE) / PAGE_SIZE;
> offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
> @@ -1418,14 +1388,9 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
> }
>
> /* Start the write request */
> - bio_init(pkt->w_bio);
> - pkt->w_bio->bi_max_vecs = PACKET_MAX_SIZE;
> + bio_reset(pkt->w_bio);
> pkt->w_bio->bi_sector = pkt->sector;
> pkt->w_bio->bi_bdev = pd->bdev;
> - pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
> - pkt->w_bio->bi_private = pkt;
> - pkt->w_bio->bi_io_vec = bvec;
> - pkt->w_bio->bi_destructor = pkt_bio_destructor;
> for (f = 0; f < pkt->frames; f++)
> if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset))
> BUG();
> --
> 1.7.7.3
>

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-09-2012, 12:08 AM
Kent Overstreet
 
Default pktcdvd: Switch to bio_kmalloc()

On Wed, Aug 08, 2012 at 03:13:59PM -0700, Tejun Heo wrote:
> Hello,
>
> On Mon, Aug 06, 2012 at 03:08:33PM -0700, Kent Overstreet wrote:
> > This is prep work for killing bi_destructor - previously, pktcdvd had
> > its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
> > necessitating its own bi_destructor implementation.
> >
> > v5: Un-reorder some functions, to make the patch easier to review
> >
> > Signed-off-by: Kent Overstreet <koverstreet@google.com>
>
> Please Cc: the maintainers. Cc'ing Peter Osterlund and keeping the
> whole body for him.

Whoops, thanks.

> Generally looks good to me. How is this tested?

Untested - no hardware for it.

>
> Thanks.
>
> > ---
> > drivers/block/pktcdvd.c | 67 +++++++++++-----------------------------------
> > 1 files changed, 16 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> > index ba66e44..ae55f08 100644
> > --- a/drivers/block/pktcdvd.c
> > +++ b/drivers/block/pktcdvd.c
> > @@ -101,6 +101,8 @@ static struct dentry *pkt_debugfs_root = NULL; /* /sys/kernel/debug/pktcdvd */
> > static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev);
> > static int pkt_remove_dev(dev_t pkt_dev);
> > static int pkt_seq_show(struct seq_file *m, void *p);
> > +static void pkt_end_io_read(struct bio *bio, int err);
> > +static void pkt_end_io_packet_write(struct bio *bio, int err);
> >
> >
> >
> > @@ -522,38 +524,6 @@ static void pkt_bio_finished(struct pktcdvd_device *pd)
> > }
> > }
> >
> > -static void pkt_bio_destructor(struct bio *bio)
> > -{
> > - kfree(bio->bi_io_vec);
> > - kfree(bio);
> > -}
> > -
> > -static struct bio *pkt_bio_alloc(int nr_iovecs)
> > -{
> > - struct bio_vec *bvl = NULL;
> > - struct bio *bio;
> > -
> > - bio = kmalloc(sizeof(struct bio), GFP_KERNEL);
> > - if (!bio)
> > - goto no_bio;
> > - bio_init(bio);
> > -
> > - bvl = kcalloc(nr_iovecs, sizeof(struct bio_vec), GFP_KERNEL);
> > - if (!bvl)
> > - goto no_bvl;
> > -
> > - bio->bi_max_vecs = nr_iovecs;
> > - bio->bi_io_vec = bvl;
> > - bio->bi_destructor = pkt_bio_destructor;
> > -
> > - return bio;
> > -
> > - no_bvl:
> > - kfree(bio);
> > - no_bio:
> > - return NULL;
> > -}
> > -
> > /*
> > * Allocate a packet_data struct
> > */
> > @@ -567,10 +537,13 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
> > goto no_pkt;
> >
> > pkt->frames = frames;
> > - pkt->w_bio = pkt_bio_alloc(frames);
> > + pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames);
> > if (!pkt->w_bio)
> > goto no_bio;
> >
> > + pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
> > + pkt->w_bio->bi_private = pkt;
> > +
> > for (i = 0; i < frames / FRAMES_PER_PAGE; i++) {
> > pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO);
> > if (!pkt->pages[i])
> > @@ -581,9 +554,12 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
> > bio_list_init(&pkt->orig_bios);
> >
> > for (i = 0; i < frames; i++) {
> > - struct bio *bio = pkt_bio_alloc(1);
> > + struct bio *bio = bio_kmalloc(GFP_KERNEL, 1);
> > if (!bio)
> > goto no_rd_bio;
> > +
> > + bio->bi_end_io = pkt_end_io_read;
> > + bio->bi_private = pkt;
> > pkt->r_bios[i] = bio;
> > }
> >
> > @@ -1111,21 +1087,15 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
> > * Schedule reads for missing parts of the packet.
> > */
> > for (f = 0; f < pkt->frames; f++) {
> > - struct bio_vec *vec;
> > -
> > int p, offset;
> > +
> > if (written[f])
> > continue;
> > +
> > bio = pkt->r_bios[f];
> > - vec = bio->bi_io_vec;
> > - bio_init(bio);
> > - bio->bi_max_vecs = 1;
> > - bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
> > - bio->bi_bdev = pd->bdev;
> > - bio->bi_end_io = pkt_end_io_read;
> > - bio->bi_private = pkt;
> > - bio->bi_io_vec = vec;
> > - bio->bi_destructor = pkt_bio_destructor;
> > + bio_reset(bio);
> > + bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
> > + bio->bi_bdev = pd->bdev;
> >
> > p = (f * CD_FRAMESIZE) / PAGE_SIZE;
> > offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
> > @@ -1418,14 +1388,9 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
> > }
> >
> > /* Start the write request */
> > - bio_init(pkt->w_bio);
> > - pkt->w_bio->bi_max_vecs = PACKET_MAX_SIZE;
> > + bio_reset(pkt->w_bio);
> > pkt->w_bio->bi_sector = pkt->sector;
> > pkt->w_bio->bi_bdev = pd->bdev;
> > - pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
> > - pkt->w_bio->bi_private = pkt;
> > - pkt->w_bio->bi_io_vec = bvec;
> > - pkt->w_bio->bi_destructor = pkt_bio_destructor;
> > for (f = 0; f < pkt->frames; f++)
> > if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset))
> > BUG();
> > --
> > 1.7.7.3
> >
>
> --
> tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-22-2012, 05:04 PM
Kent Overstreet
 
Default pktcdvd: Switch to bio_kmalloc()

This is prep work for killing bi_destructor - previously, pktcdvd had
its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
necessitating its own bi_destructor implementation.

v5: Un-reorder some functions, to make the patch easier to review

Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Peter Osterlund <petero2@telia.com>
---
drivers/block/pktcdvd.c | 67 ++++++++++++-------------------------------------
1 file changed, 16 insertions(+), 51 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index ba66e44..ae55f08 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -101,6 +101,8 @@ static struct dentry *pkt_debugfs_root = NULL; /* /sys/kernel/debug/pktcdvd */
static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev);
static int pkt_remove_dev(dev_t pkt_dev);
static int pkt_seq_show(struct seq_file *m, void *p);
+static void pkt_end_io_read(struct bio *bio, int err);
+static void pkt_end_io_packet_write(struct bio *bio, int err);



@@ -522,38 +524,6 @@ static void pkt_bio_finished(struct pktcdvd_device *pd)
}
}

-static void pkt_bio_destructor(struct bio *bio)
-{
- kfree(bio->bi_io_vec);
- kfree(bio);
-}
-
-static struct bio *pkt_bio_alloc(int nr_iovecs)
-{
- struct bio_vec *bvl = NULL;
- struct bio *bio;
-
- bio = kmalloc(sizeof(struct bio), GFP_KERNEL);
- if (!bio)
- goto no_bio;
- bio_init(bio);
-
- bvl = kcalloc(nr_iovecs, sizeof(struct bio_vec), GFP_KERNEL);
- if (!bvl)
- goto no_bvl;
-
- bio->bi_max_vecs = nr_iovecs;
- bio->bi_io_vec = bvl;
- bio->bi_destructor = pkt_bio_destructor;
-
- return bio;
-
- no_bvl:
- kfree(bio);
- no_bio:
- return NULL;
-}
-
/*
* Allocate a packet_data struct
*/
@@ -567,10 +537,13 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
goto no_pkt;

pkt->frames = frames;
- pkt->w_bio = pkt_bio_alloc(frames);
+ pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames);
if (!pkt->w_bio)
goto no_bio;

+ pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
+ pkt->w_bio->bi_private = pkt;
+
for (i = 0; i < frames / FRAMES_PER_PAGE; i++) {
pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO);
if (!pkt->pages[i])
@@ -581,9 +554,12 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
bio_list_init(&pkt->orig_bios);

for (i = 0; i < frames; i++) {
- struct bio *bio = pkt_bio_alloc(1);
+ struct bio *bio = bio_kmalloc(GFP_KERNEL, 1);
if (!bio)
goto no_rd_bio;
+
+ bio->bi_end_io = pkt_end_io_read;
+ bio->bi_private = pkt;
pkt->r_bios[i] = bio;
}

@@ -1111,21 +1087,15 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
* Schedule reads for missing parts of the packet.
*/
for (f = 0; f < pkt->frames; f++) {
- struct bio_vec *vec;
-
int p, offset;
+
if (written[f])
continue;
+
bio = pkt->r_bios[f];
- vec = bio->bi_io_vec;
- bio_init(bio);
- bio->bi_max_vecs = 1;
- bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
- bio->bi_bdev = pd->bdev;
- bio->bi_end_io = pkt_end_io_read;
- bio->bi_private = pkt;
- bio->bi_io_vec = vec;
- bio->bi_destructor = pkt_bio_destructor;
+ bio_reset(bio);
+ bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
+ bio->bi_bdev = pd->bdev;

p = (f * CD_FRAMESIZE) / PAGE_SIZE;
offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
@@ -1418,14 +1388,9 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
}

/* Start the write request */
- bio_init(pkt->w_bio);
- pkt->w_bio->bi_max_vecs = PACKET_MAX_SIZE;
+ bio_reset(pkt->w_bio);
pkt->w_bio->bi_sector = pkt->sector;
pkt->w_bio->bi_bdev = pd->bdev;
- pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
- pkt->w_bio->bi_private = pkt;
- pkt->w_bio->bi_io_vec = bvec;
- pkt->w_bio->bi_destructor = pkt_bio_destructor;
for (f = 0; f < pkt->frames; f++)
if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset))
BUG();
--
1.7.12

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-22-2012, 07:55 PM
Tejun Heo
 
Default pktcdvd: Switch to bio_kmalloc()

(cc'ing Jiri, hi!)

On Wed, Aug 22, 2012 at 10:04:01AM -0700, Kent Overstreet wrote:
> This is prep work for killing bi_destructor - previously, pktcdvd had
> its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
> necessitating its own bi_destructor implementation.
>
> v5: Un-reorder some functions, to make the patch easier to review
>
> Signed-off-by: Kent Overstreet <koverstreet@google.com>
> CC: Peter Osterlund <petero2@telia.com>

Apart from bio_reset() not resetting bi_end_io and bi_private, this
looks fine to me but lack of testing makes me a bit hesitant to ack
it.

Peter doesn't want to be the maintainer of pktcdvd anymore. Maybe
Jiri can be tricked into taking this one too?

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-28-2012, 05:37 PM
Kent Overstreet
 
Default pktcdvd: Switch to bio_kmalloc()

This is prep work for killing bi_destructor - previously, pktcdvd had
its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
necessitating its own bi_destructor implementation.

v5: Un-reorder some functions, to make the patch easier to review

Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Peter Osterlund <petero2@telia.com>
---
drivers/block/pktcdvd.c | 52 +++++++------------------------------------------
1 file changed, 7 insertions(+), 45 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index ba66e44..2e7de7a 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -522,38 +522,6 @@ static void pkt_bio_finished(struct pktcdvd_device *pd)
}
}

-static void pkt_bio_destructor(struct bio *bio)
-{
- kfree(bio->bi_io_vec);
- kfree(bio);
-}
-
-static struct bio *pkt_bio_alloc(int nr_iovecs)
-{
- struct bio_vec *bvl = NULL;
- struct bio *bio;
-
- bio = kmalloc(sizeof(struct bio), GFP_KERNEL);
- if (!bio)
- goto no_bio;
- bio_init(bio);
-
- bvl = kcalloc(nr_iovecs, sizeof(struct bio_vec), GFP_KERNEL);
- if (!bvl)
- goto no_bvl;
-
- bio->bi_max_vecs = nr_iovecs;
- bio->bi_io_vec = bvl;
- bio->bi_destructor = pkt_bio_destructor;
-
- return bio;
-
- no_bvl:
- kfree(bio);
- no_bio:
- return NULL;
-}
-
/*
* Allocate a packet_data struct
*/
@@ -567,7 +535,7 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
goto no_pkt;

pkt->frames = frames;
- pkt->w_bio = pkt_bio_alloc(frames);
+ pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames);
if (!pkt->w_bio)
goto no_bio;

@@ -581,9 +549,10 @@ static struct packet_data *pkt_alloc_packet_data(int frames)
bio_list_init(&pkt->orig_bios);

for (i = 0; i < frames; i++) {
- struct bio *bio = pkt_bio_alloc(1);
+ struct bio *bio = bio_kmalloc(GFP_KERNEL, 1);
if (!bio)
goto no_rd_bio;
+
pkt->r_bios[i] = bio;
}

@@ -1111,21 +1080,17 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
* Schedule reads for missing parts of the packet.
*/
for (f = 0; f < pkt->frames; f++) {
- struct bio_vec *vec;
-
int p, offset;
+
if (written[f])
continue;
+
bio = pkt->r_bios[f];
- vec = bio->bi_io_vec;
- bio_init(bio);
- bio->bi_max_vecs = 1;
+ bio_reset(bio);
bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
bio->bi_bdev = pd->bdev;
bio->bi_end_io = pkt_end_io_read;
bio->bi_private = pkt;
- bio->bi_io_vec = vec;
- bio->bi_destructor = pkt_bio_destructor;

p = (f * CD_FRAMESIZE) / PAGE_SIZE;
offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
@@ -1418,14 +1383,11 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
}

/* Start the write request */
- bio_init(pkt->w_bio);
- pkt->w_bio->bi_max_vecs = PACKET_MAX_SIZE;
+ bio_reset(pkt->w_bio);
pkt->w_bio->bi_sector = pkt->sector;
pkt->w_bio->bi_bdev = pd->bdev;
pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
pkt->w_bio->bi_private = pkt;
- pkt->w_bio->bi_io_vec = bvec;
- pkt->w_bio->bi_destructor = pkt_bio_destructor;
for (f = 0; f < pkt->frames; f++)
if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset))
BUG();
--
1.7.12

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-28-2012, 08:32 PM
Tejun Heo
 
Default pktcdvd: Switch to bio_kmalloc()

On Tue, Aug 28, 2012 at 10:37:31AM -0700, Kent Overstreet wrote:
> This is prep work for killing bi_destructor - previously, pktcdvd had
> its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
> necessitating its own bi_destructor implementation.

How was this tested?

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-28-2012, 10:24 PM
Kent Overstreet
 
Default pktcdvd: Switch to bio_kmalloc()

On Tue, Aug 28, 2012 at 01:32:47PM -0700, Tejun Heo wrote:
> On Tue, Aug 28, 2012 at 10:37:31AM -0700, Kent Overstreet wrote:
> > This is prep work for killing bi_destructor - previously, pktcdvd had
> > its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
> > necessitating its own bi_destructor implementation.
>
> How was this tested?

Hasn't been yet, but I'm getting a new test machine in the next day or
so (and also the patch is a lot smaller since the bio_reset() changes
you suggested).

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-28-2012, 11:19 PM
Jiri Kosina
 
Default pktcdvd: Switch to bio_kmalloc()

On Wed, 22 Aug 2012, Tejun Heo wrote:

> (cc'ing Jiri, hi!)

Hi there!

> On Wed, Aug 22, 2012 at 10:04:01AM -0700, Kent Overstreet wrote:
> > This is prep work for killing bi_destructor - previously, pktcdvd had
> > its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
> > necessitating its own bi_destructor implementation.
> >
> > v5: Un-reorder some functions, to make the patch easier to review
> >
> > Signed-off-by: Kent Overstreet <koverstreet@google.com>
> > CC: Peter Osterlund <petero2@telia.com>
>
> Apart from bio_reset() not resetting bi_end_io and bi_private, this
> looks fine to me but lack of testing makes me a bit hesitant to ack
> it.
>
> Peter doesn't want to be the maintainer of pktcdvd anymore. Maybe
> Jiri can be tricked into taking this one too?

Gah, seems like the floppy saga is going to cause me some more trouble in
the future ...

Well, I definitely have hardware back at home that supports this, so I can
take it. Doesn't seem to be heavy traffic code either.

Peter, ok with that? Also, how was this usually fed upstream -- through
Jens' tree?

--
Jiri Kosina
SUSE Labs

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-29-2012, 04:35 AM
Peter Osterlund
 
Default pktcdvd: Switch to bio_kmalloc()

On Tue, 28 Aug 2012, Jiri Kosina wrote:


On Wed, 22 Aug 2012, Tejun Heo wrote:


(cc'ing Jiri, hi!)


Hi there!


On Wed, Aug 22, 2012 at 10:04:01AM -0700, Kent Overstreet wrote:

This is prep work for killing bi_destructor - previously, pktcdvd had
its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
necessitating its own bi_destructor implementation.

v5: Un-reorder some functions, to make the patch easier to review

Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Peter Osterlund <petero2@telia.com>


Apart from bio_reset() not resetting bi_end_io and bi_private, this
looks fine to me but lack of testing makes me a bit hesitant to ack
it.

Peter doesn't want to be the maintainer of pktcdvd anymore. Maybe
Jiri can be tricked into taking this one too?


Gah, seems like the floppy saga is going to cause me some more trouble in
the future ...

Well, I definitely have hardware back at home that supports this, so I can
take it. Doesn't seem to be heavy traffic code either.

Peter, ok with that? Also, how was this usually fed upstream -- through
Jens' tree?


Yes, I am ok with that.

--
Peter Osterlund - peterosterlund2@gmail.com, petero2@comhem.se
http://web.comhem.se/petero2home

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 02:39 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org