This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] copy_file_range: New function to copy file data



On 18/12/2017 18:19, Florian Weimer wrote:
> On 12/18/2017 09:04 PM, Adhemerval Zanella wrote:
> 
>> I am seeing a lot of failures from tst-copy_file_range on both x86_64 and i686
>> when using the fallback implementation (io/copy_file_range.c) due the fact
>> write is returning EFBIG (I attached the test output):
> 
> Which file system is this?

It is a default ubuntu 16 installation: /tmp is ext4 (rw,relatime,errors=remount-ro,data=ordered)
with kernel 4.4.0-71-generic

> 
>>
>> [...]
>> openat(AT_FDCWD, "/tmp/tst-copy_file_range-in-mT9I6j", O_RDWR) = 3
>> ftruncate(3, 0)                         = 0
>> openat(AT_FDCWD, "/tmp/tst-copy_file_range-out-0RbGw2", O_RDWR) = 4
>> ftruncate(4, 0)
>> [...]
>> fcntl(4, F_GETFL)                       = 0x8002 (flags O_RDWR|O_LARGEFILE)
>> read(3, "k2dft\31*b#F=P.AyuQ[\22M\2\37\21f\0243\20\r\177\33Ak"..., 8192) = 8192
>> write(4, "k2dft\31*b#F=P.AyuQ[\22M\2\37\21f\0243\20\r\177\33Ak"..., 8192) = -1 EFBIG (File too large)
>> lseek(3, -8192, SEEK_CUR)               = 0
>> write(1, "tst-copy_file_range.c:285: numer"..., 54tst-copy_file_range.c:285: numeric comparison failure
>> [...]
> 
> It would be interesting to compare this with the real copy_file_range system call.  I don't think it remaps EFBIG, so this might also apply there, too.

I also does not, running  example to trigger EFBIG on a 4.13.0-19-generic I see:

[...]
  int fin  = open ("/tmp/file.in",  O_RDWR | O_CREAT | O_TRUNC, 0600);
  assert (fin != -1);
  int fout = open ("/tmp/file.out", O_RDWR | O_CREAT | O_TRUNC, 0600);
  assert (fout != -1);

  char buffer[8192];
  const size_t size = 8192;
  memset (buffer, 0xcc, size);
  assert (pwrite (fin, buffer, size, 0) == size);

  ssize_t ret = copy_file_range (fin, 0, fout, &(__off64_t) { INT32_MAX }, size, 0);
[...]

strace ...
[...]
openat(AT_FDCWD, "/tmp/file.in", O_RDWR|O_CREAT|O_TRUNC, 0600) = 3
openat(AT_FDCWD, "/tmp/file.out", O_RDWR|O_CREAT|O_TRUNC, 0600) = 4
pwrite64(3, "\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314\314"..., 8192, 0) = 8192
copy_file_range(3, NULL, 4, [2147483647], 8192, 0) = -1 EFBIG (File too large)
fstat64(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 5), ...}) = 0
[...]

> 
>> I also noted it does not provided a non-LFS version and it a good way forward
>> imho, however I think we need to explicit handle the case where a non-LFS
>> invocation tries to use copy_file_range in a non-supported way.  For instance
>> the snippet:
>>
>> [...]
>> int fin  = open ("/tmp/file.in",  O_RDWR | O_CREAT | O_TRUNC, 0600);                                                                                                                                                                                                                                                                                          int fout = open ("/tmp/file.out", O_RDWR | O_CREAT | O_TRUNC, 0600);                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 char buffer[8192] = { 0xcc
>> };                                                                                                                                                                       const size_t size = 8192;                                                                                                                                                                                                                                                                                                                           pwrite (fin, buffer, size, 0);                                                                                                                                                                                                                                                                                                                                   copy_file_range (fin, 0, fout, &(__off64_t) { INT32_MAX }, size, 0);
>> [...]
>>
>> Will again return EFBIG.
> 
> Hmm.  I think this is just the EFBIG problem.  I'd be more concerned about EBADF here.
> 
>> We have some options as 1. handle EFBIG
>> as an expected retuned error, 2. do not declare copy_file_range for
>> !__USE_FILE_OFFSET64, 3. add a dummy implementation for non-LFS
>> (which return ENOSYS).
> 
> I'd like to reproduce this with the file system you used on a kernel with a copy_file_range system call, and see what the system call does there.
> 
>>> +  /* Main copying loop.  The buffer size is arbitrary and is a
>>> +     trade-off between stack size consumption, cache usage, and
>>> +     amortization of system call overhead.  */
>>> +  size_t copied = 0;
>>> +  char buf[8192];
>>
>> Do we you have any numbers with shorter sizes? Maybe
> 
> Sorry, could you expand?
> 
> Choosing buffer sizes is notoriously difficult, I'm afraid.

I meant performance number, I am kind worries about the buffer size requirement
(although it align to BUFSIZ).

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]