On Tue, Nov 22, 2016 at 01:38:46AM -0800, Christoph Hellwig wrote:
> On Fri, Nov 18, 2016 at 05:29:06PM -0800, Liu Bo wrote:
> > On Wed, Nov 16, 2016 at 01:52:08PM +0100, Christoph Hellwig wrote:
> > > Pass the full bio to the decompression routines and use bio iterators
> > > to iterate over the data in the bio.
> > 
> > One question below,
> 
> It would be nice to cut down the email to actually find your question
> without running through hundreds+ quoted lines.

Sure :)

> 
> > >   /* copy bytes from the working buffer into the pages */
> > >   while (working_bytes > 0) {
> > > -         bytes = min(PAGE_SIZE - *pg_offset,
> > > -                     PAGE_SIZE - buf_offset);
> > > +         bytes = min_t(unsigned long, bvec.bv_len,
> > > +                         PAGE_SIZE - buf_offset);
> > >           bytes = min(bytes, working_bytes);
> > > -         kaddr = kmap_atomic(page_out);
> > > -         memcpy(kaddr + *pg_offset, buf + buf_offset, bytes);
> > > +
> > > +         kaddr = kmap_atomic(bvec.bv_page);
> > > +         memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes);
> > 
> > This doesn't seem to be right, 'bvec.bv_offset' is not updated in the
> > following bio_advance(bio, bytes),
> 
> Good spot - and this means xfstests doesn't cover this area very
> well :(

The decompress part starts after checking checksum stored in btrfs, I'm
wondering if fio --verify could detect it.

> 
> > shouldn't it be 
> > 'kaddr + bvec.bv_offset + bio->bi_iter.bi_bvec_done'?
> 
> No, we just need to get a new bvec using bio_iter_iovec after
> the call to bio_advance.

I see, that's great.

Thanks,

-liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to