Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-09 Thread Tejun Heo
Hello, On Wed, Aug 8, 2012 at 11:06 PM, Kent Overstreet wrote: > On Wed, Aug 08, 2012 at 11:00:19PM -0700, Tejun Heo wrote: >> Anyways, understood. Can you *please* put some comment what bits are >> being preserved across reset then? Things like this aren't obvious at >> all and need ample

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-09 Thread Kent Overstreet
On Wed, Aug 08, 2012 at 11:00:19PM -0700, Tejun Heo wrote: > Anyways, understood. Can you *please* put some comment what bits are > being preserved across reset then? Things like this aren't obvious at > all and need ample explanation. I did, in the header: #define BIO_RESET_BITS 12 /*

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-09 Thread Tejun Heo
Hello, On Wed, Aug 08, 2012 at 05:07:11PM -0700, Kent Overstreet wrote: > > > +void bio_reset(struct bio *bio) > > > +{ > > > + unsigned long flags = bio->bi_flags & (~0UL << BIO_RESET_BITS); > > > > How many flags are we talking about? If there aren't too many, I'd > > prefer explicit

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-09 Thread Tejun Heo
Hello, On Wed, Aug 08, 2012 at 05:07:11PM -0700, Kent Overstreet wrote: +void bio_reset(struct bio *bio) +{ + unsigned long flags = bio-bi_flags (~0UL BIO_RESET_BITS); How many flags are we talking about? If there aren't too many, I'd prefer explicit BIO_FLAGS_PRESERVED or

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-09 Thread Kent Overstreet
On Wed, Aug 08, 2012 at 11:00:19PM -0700, Tejun Heo wrote: Anyways, understood. Can you *please* put some comment what bits are being preserved across reset then? Things like this aren't obvious at all and need ample explanation. I did, in the header: #define BIO_RESET_BITS 12 /*

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-09 Thread Tejun Heo
Hello, On Wed, Aug 8, 2012 at 11:06 PM, Kent Overstreet koverstr...@google.com wrote: On Wed, Aug 08, 2012 at 11:00:19PM -0700, Tejun Heo wrote: Anyways, understood. Can you *please* put some comment what bits are being preserved across reset then? Things like this aren't obvious at all and

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-08 Thread Kent Overstreet
On Wed, Aug 08, 2012 at 03:11:29PM -0700, Tejun Heo wrote: > Hello, > > On Mon, Aug 06, 2012 at 03:08:32PM -0700, Kent Overstreet wrote: > > Reusing bios is something that's been highly frowned upon in the past, > > but driver code keeps doing it anyways. If it's going to happen anyways, > > we

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-08 Thread Tejun Heo
Hello, On Mon, Aug 06, 2012 at 03:08:32PM -0700, Kent Overstreet wrote: > Reusing bios is something that's been highly frowned upon in the past, > but driver code keeps doing it anyways. If it's going to happen anyways, > we should provide a generic method. > > This'll help with getting rid of

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-08 Thread Tejun Heo
Hello, On Mon, Aug 06, 2012 at 03:08:32PM -0700, Kent Overstreet wrote: Reusing bios is something that's been highly frowned upon in the past, but driver code keeps doing it anyways. If it's going to happen anyways, we should provide a generic method. This'll help with getting rid of

Re: [PATCH v5 03/12] block: Add bio_reset()

2012-08-08 Thread Kent Overstreet
On Wed, Aug 08, 2012 at 03:11:29PM -0700, Tejun Heo wrote: Hello, On Mon, Aug 06, 2012 at 03:08:32PM -0700, Kent Overstreet wrote: Reusing bios is something that's been highly frowned upon in the past, but driver code keeps doing it anyways. If it's going to happen anyways, we should

[PATCH v5 03/12] block: Add bio_reset()

2012-08-06 Thread Kent Overstreet
Reusing bios is something that's been highly frowned upon in the past, but driver code keeps doing it anyways. If it's going to happen anyways, we should provide a generic method. This'll help with getting rid of bi_destructor - drivers/block/pktcdvd.c was open coding it, by doing a bio_init()

[PATCH v5 03/12] block: Add bio_reset()

2012-08-06 Thread Kent Overstreet
Reusing bios is something that's been highly frowned upon in the past, but driver code keeps doing it anyways. If it's going to happen anyways, we should provide a generic method. This'll help with getting rid of bi_destructor - drivers/block/pktcdvd.c was open coding it, by doing a bio_init()