Actually, the broken zero-copy for data is discouraging. I think the way we could do is rearrange the layout of journal entry, just like message we make journal header and payload(extra encoding bytes without data), then data and footer. I think it should be a easy adapt change? On Fri, Nov 20, 2015 at 9:29 PM, Haomai Wang <haomaiwang@xxxxxxxxx> wrote: > On Fri, Nov 20, 2015 at 9:08 PM, Sage Weil <sage@xxxxxxxxxxxx> wrote: >> On Fri, 20 Nov 2015, Haomai Wang wrote: >>> On Fri, Nov 20, 2015 at 7:41 PM, Sage Weil <sage@xxxxxxxxxxxx> wrote: >>> > On Fri, 20 Nov 2015, changtao381 wrote: >>> >> Hi All, >>> >> >>> >> Thanks for you apply! >>> >> >>> >> If directioIO + async IO requirement that alignment, it shouldn't aligned by PAGE for each journal entry. >>> >> For it may write many entries of journal once time >>> > >>> > We also want to avoid copying the data around in memory to change the >>> > alignment. The messenger takes care to read data off the wire into >>> > buffers with the correct alignment so that we can later use them for >>> > direct-io. >>> > >>> > If you're worried about the small io case, I think this is just a matter >>> > of setting a threshold for small ios so that we don't bother with all of >>> > the padding when the memory copy isn't that expensive. But... given that >>> > we have a header *and* footer in the journal format and almost all IOs are >>> > 4k multiples I think it'd save you a single 4k block at most. >>> > >>> > (Also, I thought we already did something like this, but perhaps not!) >>> >>> Hmm, based on our recently test, the data from messenger is aligned. >>> But the encoded data(pglog, transaction) will make thing worse, like >>> PR(https://github.com/ceph/ceph/pull/6368) solved, we even will get 14 >>> ptr in the bufferlist which passed into filejournal before. So it make >>> we rebuild each time within filejournal thread. Like this >>> PR(https://github.com/ceph/ceph/pull/6484), we try to make it rebuild >>> not in filejournal thread which is single. >> >> buffer::list::rebuild_page_aligned() should only copy/rebuild ptrs that >> are unaligned, and leave aligned ones untouched. It looks like the >> journal code is already doing this? > > Yes or not, for example we have a bufferlist contains 2 ptrs, the > first is unaligned, the second is aligned. But the current impl will > ignore the second alignment fact. Look at the code: > > void buffer::list::rebuild_aligned_size_and_memory(unsigned align_size, > unsigned align_memory) > { > ........ > list unaligned; > unsigned offset = 0; > do { > /*cout << " segment " << (void*)p->c_str() > << " offset " << ((unsigned long)p->c_str() & (align - 1)) > << " length " << p->length() << " " << (p->length() & > (align - 1)) > << " overall offset " << offset << " " << (offset & (align - 1)) > << " not ok" << std::endl; > */ > offset += p->length(); > unaligned.push_back(*p); > _buffers.erase(p++); > } while (p != _buffers.end() && > (!p->is_aligned(align_memory) || > !p->is_n_align_sized(align_size) || > (offset % align_size))); > ((((((((((((( it will check offset alignment, so won't continues after > meeting the first unalign ptr )))))))))))))) > > if (!(unaligned.is_contiguous() && > unaligned._buffers.front().is_aligned(align_memory))) { > ptr nb(buffer::create_aligned(unaligned._len, align_memory)); > unaligned.rebuild(nb); > _memcopy_count += unaligned._len; > } > _buffers.insert(p, unaligned._buffers.front()); > } > } > > >> >> sage > > > > -- > Best Regards, > > Wheat -- Best Regards, Wheat -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
