On Thu, Aug 09, 2012 at 10:37:00AM -0700, Tejun Heo wrote:
> Hello,
>
> On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
> > bcache creates large bios internally, and then splits them according to
> > the device requirements before it sends them down. If a lower level
> > device
On Thu, Aug 09, 2012 at 10:37:00AM -0700, Tejun Heo wrote:
Hello,
On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
bcache creates large bios internally, and then splits them according to
the device requirements before it sends them down. If a lower level
device tries to
Tejun,
On Thu, Aug 9, 2012 at 12:01 AM, Tejun Heo wrote:
> Hello,
>
> On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:
>> You are changing the meaning of __bio_clone() here. In old code, the
>> number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
>> code, you are
Kent,
>> --
>> You are changing the meaning of __bio_clone() here. In old code, the
>> number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
>> code, you are mapping bio_src's bi_iovec[bi_idx] to bio_dests
>> bi_iovec[0] and also restricting the number of allocated io_vecs of
>>
Hello,
On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
> bcache creates large bios internally, and then splits them according to
> the device requirements before it sends them down. If a lower level
> device tries to clone the bio, and the original bio had more than
>
Hello,
On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:
> You are changing the meaning of __bio_clone() here. In old code, the
> number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
> code, you are mapping bio_src's bi_iovec[bi_idx] to bio_dests
> bi_iovec[0] and
Hello,
On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:
You are changing the meaning of __bio_clone() here. In old code, the
number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
code, you are mapping bio_src's bi_iovec[bi_idx] to bio_dests
bi_iovec[0] and also
Hello,
On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
bcache creates large bios internally, and then splits them according to
the device requirements before it sends them down. If a lower level
device tries to clone the bio, and the original bio had more than
BIO_MAX_PAGES,
Kent,
--
You are changing the meaning of __bio_clone() here. In old code, the
number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
code, you are mapping bio_src's bi_iovec[bi_idx] to bio_dests
bi_iovec[0] and also restricting the number of allocated io_vecs of
the clone.
Tejun,
On Thu, Aug 9, 2012 at 12:01 AM, Tejun Heo t...@kernel.org wrote:
Hello,
On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:
You are changing the meaning of __bio_clone() here. In old code, the
number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
code, you
On Wed, Aug 8, 2012 at 8:19 PM, Kent Overstreet wrote:
> In particular, if this change breaks anything then the new bio_split()
> _will_ break things.
>
> We need to be clear about our interfaces; in this case bi_idx and
> bi_vcnt, in particular. Either this is a safe change, or it's not. If
> no
On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:
> Tejun,
>
> This is changing the semantics of the clone. Sorry, I missed this
> thread and replied separately. But anyway, replying it again here:
>
>
> On Wed, Aug 8, 2012 at 4:28 PM, Tejun Heo wrote:
> > On Mon, Aug 06, 2012 at
On Wed, Aug 08, 2012 at 04:30:07PM -0700, Tejun Heo wrote:
> Hello,
>
> On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
> > @@ -459,10 +460,10 @@ void __bio_clone(struct bio *bio, struct bio *bio_src)
> > bio->bi_sector = bio_src->bi_sector;
> > bio->bi_bdev =
Tejun,
This is changing the semantics of the clone. Sorry, I missed this
thread and replied separately. But anyway, replying it again here:
On Wed, Aug 8, 2012 at 4:28 PM, Tejun Heo wrote:
> On Mon, Aug 06, 2012 at 07:16:33PM -0400, Mikulas Patocka wrote:
>> Hi Kent
>>
>> When you change the
Hello,
On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
> @@ -459,10 +460,10 @@ void __bio_clone(struct bio *bio, struct bio *bio_src)
> bio->bi_sector = bio_src->bi_sector;
> bio->bi_bdev = bio_src->bi_bdev;
> bio->bi_flags |= 1 << BIO_CLONED;
> +
On Mon, Aug 06, 2012 at 07:16:33PM -0400, Mikulas Patocka wrote:
> Hi Kent
>
> When you change the semantics of an exported function, rename that
> function. There may be external modules that use __bio_clone and this
> change could silently introduce bugs in them.
>
> Otherwise, the patchset
On Mon, Aug 06, 2012 at 07:16:33PM -0400, Mikulas Patocka wrote:
Hi Kent
When you change the semantics of an exported function, rename that
function. There may be external modules that use __bio_clone and this
change could silently introduce bugs in them.
Otherwise, the patchset looks
Hello,
On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
@@ -459,10 +460,10 @@ void __bio_clone(struct bio *bio, struct bio *bio_src)
bio-bi_sector = bio_src-bi_sector;
bio-bi_bdev = bio_src-bi_bdev;
bio-bi_flags |= 1 BIO_CLONED;
+ bio-bi_flags = ~(1
Tejun,
This is changing the semantics of the clone. Sorry, I missed this
thread and replied separately. But anyway, replying it again here:
On Wed, Aug 8, 2012 at 4:28 PM, Tejun Heo t...@kernel.org wrote:
On Mon, Aug 06, 2012 at 07:16:33PM -0400, Mikulas Patocka wrote:
Hi Kent
When you
On Wed, Aug 08, 2012 at 04:30:07PM -0700, Tejun Heo wrote:
Hello,
On Mon, Aug 06, 2012 at 03:08:41PM -0700, Kent Overstreet wrote:
@@ -459,10 +460,10 @@ void __bio_clone(struct bio *bio, struct bio *bio_src)
bio-bi_sector = bio_src-bi_sector;
bio-bi_bdev = bio_src-bi_bdev;
On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:
Tejun,
This is changing the semantics of the clone. Sorry, I missed this
thread and replied separately. But anyway, replying it again here:
On Wed, Aug 8, 2012 at 4:28 PM, Tejun Heo t...@kernel.org wrote:
On Mon, Aug 06, 2012
On Wed, Aug 8, 2012 at 8:19 PM, Kent Overstreet koverstr...@google.com wrote:
In particular, if this change breaks anything then the new bio_split()
_will_ break things.
We need to be clear about our interfaces; in this case bi_idx and
bi_vcnt, in particular. Either this is a safe change, or
Hi Kent
When you change the semantics of an exported function, rename that
function. There may be external modules that use __bio_clone and this
change could silently introduce bugs in them.
Otherwise, the patchset looks fine.
Mikulas
On Mon, 6 Aug 2012, Kent Overstreet wrote:
> bcache
bcache creates large bios internally, and then splits them according to
the device requirements before it sends them down. If a lower level
device tries to clone the bio, and the original bio had more than
BIO_MAX_PAGES, the clone will fail unecessarily.
We can fix this by only cloning the bio
bcache creates large bios internally, and then splits them according to
the device requirements before it sends them down. If a lower level
device tries to clone the bio, and the original bio had more than
BIO_MAX_PAGES, the clone will fail unecessarily.
We can fix this by only cloning the bio
Hi Kent
When you change the semantics of an exported function, rename that
function. There may be external modules that use __bio_clone and this
change could silently introduce bugs in them.
Otherwise, the patchset looks fine.
Mikulas
On Mon, 6 Aug 2012, Kent Overstreet wrote:
bcache
26 matches
Mail list logo
