Bug 806055 - "cp -a" should cache attr/fscreate
"cp -a" should cache attr/fscreate
Status: CLOSED UPSTREAM
Product: Fedora
Classification: Fedora
Component: coreutils (Show other bugs)
rawhide
Unspecified Linux
medium Severity medium
: ---
: ---
Assigned To: Ondrej Vasik
Fedora Extras Quality Assurance
: FutureFeature, Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-22 14:52 EDT by John Reiser
Modified: 2013-01-10 06:36 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-01-10 06:36:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description John Reiser 2012-03-22 14:52:40 EDT
Description of problem: "cp -a" wastes time by writing /proc/self/task/<tid>/attr/fscreate for every file, even though changes are rare.


Version-Release number of selected component (if applicable):
coreutils-8.15-6.fc17.x86_64


How reproducible: every time


Steps to Reproduce:
1. strace -o strace.out cp -a source_tree dest_tree
2. Inspect strace.out for fscreate
3.
  
Actual results: For every file copied there is a sequence similar to:
-----
gettid()                                = 2261
open("/proc/self/task/2261/attr/fscreate", O_RDWR) = 3
write(3, "unconfined_u:object_r:user_tmp_t"..., 36) = 36
close(3)                                = 0
-----

Expected results: Cache the things that do not change from file to file.  gettid() *NEVER* changes [unless cp is modified to become multi-threaded].  The SELinux attributes probably change very rarely, especially for files within the same directory.


Additional info:
Comment 1 Ondrej Oprala 2012-12-13 11:00:42 EST
Hi,
gettid () is never called directly from coreutils code, it's called by lstat ()
when SELinux is enabled. So this can't be fixed/changed in coreutils code.

Closing WONTFIX.
Comment 2 John Reiser 2012-12-13 12:27:03 EST
That analysis is incorrect.  lstat never calls gettid.  What does call gettid is this traceback [detect 186==$rax at 'syscall' instruction in syscall() routine]:
Breakpoint 4, syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
38		syscall			/* Do the system call.  */
(gdb) bt
#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x00000030f1c0ad0c in gettid () at procattr.c:13
#2  setprocattrcon_raw (
    context=0x820950 "unconfined_u:object_r:user_home_t:s0", 
    attr=attr@entry=0x30f1c172a5 "fscreate", pid=0x0) at procattr.c:102
#3  0x00000030f1c0ae21 in setprocattrcon (context=<optimized out>, 
    attr=attr@entry=0x30f1c172a5 "fscreate", pid=0x0) at procattr.c:138
#4  0x00000030f1c0af5c in setfscreatecon (c=<optimized out>) at procattr.c:183
#5  0x0000000000409047 in copy_internal (
    src_name=src_name@entry=0x7fffffffe3e9 "foo.dir", 
    dst_name=dst_name@entry=0x8208f0 "bar.dir/foo.dir", new_dst=0x0, 
    device=device@entry=0x0, ancestors=ancestors@entry=0x0, 
    x=x@entry=0x7fffffffdeb0, command_line_arg=command_line_arg@entry=0x1, 
    first_dir_created_per_command_line_arg=first_dir_created_per_command_line_arg@entry=0x7fffffffdd2f, copy_into_self=copy_into_self@entry=0x7fffffffdd8f, 
    rename_succeeded=rename_succeeded@entry=0x0) at copy.c:2139
#6  0x00000000004092ec in copy (
    src_name=src_name@entry=0x7fffffffe3e9 "foo.dir", 
    dst_name=dst_name@entry=0x8208f0 "bar.dir/foo.dir", 
    nonexistent_dst=<optimized out>, options=options@entry=0x7fffffffdeb0, 
    copy_into_self=copy_into_self@entry=0x7fffffffdd8f, 
    rename_succeeded=rename_succeeded@entry=0x0) at copy.c:2619
#7  0x000000000040498f in do_copy (n_files=0x1, file=0x7fffffffe0b8, 
    target_directory=0x7fffffffe3f1 "bar.dir", target_directory@entry=0x0, 
    no_target_directory=no_target_directory@entry=0x0, 
    x=x@entry=0x7fffffffdeb0) at cp.c:706
#8  0x00000000004036fc in main (argc=0x4, argv=0x7fffffffe0a8) at cp.c:1207

with the code (corresponding to #5 in the traceback):
copy_internal (
   <<snip>>
      if (0 <= lgetfilecon (src_name, &con))
        {
          if (setfscreatecon (con) < 0)

and this is where caching should be applied.  Compare the current length and value of 'con' to the previous 'con', and do not call setfscreatecon() if they are the same.

The problem persists as originally described; re-open this report.
Comment 3 Ondrej Oprala 2012-12-19 06:49:01 EST
Oops, it seems you're right.
Terribly sorry, I'll look into it.
Comment 4 Ondrej Oprala 2013-01-10 06:36:47 EST
Resolved in libselinux (patch already in rawhide). Relevant thread: http://lists.gnu.org/archive/html/coreutils/2013-01/msg00026.html

Note You need to log in before you can comment on or make changes to this bug.