Ilya Dryomov <idryomov@xxxxxxxxx> writes: > On Tue, Oct 9, 2018 at 12:47 PM Luis Henriques <lhenriques@xxxxxxxx> wrote: >> >> Hi, >> >> finally, here's a new iteration of my copy_file_range patchset. I've an >> extra patch (not included in this RFC) that adds tracepoints to this >> syscall. I wanted to know if that's something we would like to start >> including in the kernel cephfs client. I can prepare a new rev that >> includes this extra patch. > > No, if we start introducing tracepoints, it'll have to be throught > libceph, rbd and ceph, replacing some of the douts. Tracepoints in > some places and douts in other places is a no go. On top of that there > is the whole tracepoint ABI stability mess, although it's less of an > issue for individual filesystems... > > In any case it doesn't belong in this series. > First of all, thanks a lot for your time reviewing this patchset. I've skimmed through your comments and I believe they all make perfect sense. I'll go through all of them and prepare a new revision in the next few days. [ I still need to revisit those Op flags as I don't understand why I assumed they would both be using CEPH_OSD_OP_FLAG_FADVISE_*. I thought I saw a similar usage in the ceph code, but a quick grep didn't show anything. ] Regarding tracepoints, I agree that having both dynamic debug (dout) and tracepoints isn't a great idea. My preference would be to move to tracepoints but it would take a while to visit all the douts and come up with a set of patches that would convert the relevant ones to tracepoints (I'm sure there would be a lot of douts that could actually be dropped). Anyway, do you think it's worth opening a feature request so that some day this could be done? Or would you rather continue using dynamic debugging only? Cheers, -- Luis >> >> And here's the changelog since v4: >> >> - Complete rewrite of ceph_copy_file_range function. Now the copy loop >> includes only the remote object copies, while do_splice_direct is >> invoked (at most) twice -- before and after object copy loop. >> >> - Check sizes after every put/get caps cycle >> >> - Added new checks to ensure the files being copied have the same >> layouts >> >> Changes since v3: >> >> - release/get caps before doing the do_splice_direct calls >> >> - if an error occurs after data has been copied already, return the >> amount of bytes already copied instead of the error >> >> - fix oid init/destroy (was broken since a pre-v1 version of the patch) > > CephFS object names are bounded, so it uses a non-allocating version, > ceph_oid_printf(). Calling ceph_oid_destroy() is not necessary. > > Thanks, > > Ilya >
