On Tue, Aug 21, 2018 at 5:21 PM Luis Henriques <lhenriques@xxxxxxxx> wrote: > > "Yan, Zheng" <ukernel@xxxxxxxxx> writes: > ... > >> + ret = ceph_get_caps(dst_ci, CEPH_CAP_FILE_WR, > >> + CEPH_CAP_FILE_BUFFER, > >> + endoff, &got, NULL); > >> + if (ret < 0) > >> + goto out; > >> + > > > > Getting FILE_RD caps for src inode? I think adding another > > ceph_get_caps() call here may cause deadlock (shouldn't hold any cap > > while waiting). > > I thought about that initially, but since I'm using do_splice_direct for > doing any manual copies I assumed that this would eventually be handled > somewhere else. Because we don't really FILE_RD if we're doing only > full (remote) object copies, do we? > We still need to get FILE_RD for full object copies. Because other client may have dirty data in page cache. we need to until the dirty data get flushed > ... > > >> +out_caps: > >> + ceph_put_cap_refs(dst_ci, got); > >> +out: > >> + ceph_free_cap_flush(prealloc_cf); > >> + > >> + return ret; > >> +} > > > > There is no i_size check in above function. I think some corner cases > > may not be handled properly > > Do you have any specific case in mind? I'm checking the EOF for the > source file: Sorry I missed this. > > + size = i_size_read(src_inode); > + /* Don't copy beyond source file EOF */ > + if (src_off + len > size) > + len = (size - src_off); > > and the final destination file size: If other client is writing to the source inode, local i_size can be stale. Maybe we can use do_splice_direct(... original copy length..) for the copy-beyond-eof case. See striped_read()'s 'checkeof' for more information. Regards Yan, Zheng > > + size = i_size_read(dst_inode); > + endoff = dst_off + len; > + ret = inode_newsize_ok(dst_inode, endoff); > + if (ret) > + goto out; > > But maybe I misunderstood your question. > > Cheers, > -- > Luis
