Bug 806055

Summary: "cp -a" should cache attr/fscreate
Product: [Fedora] Fedora Reporter: John Reiser <jreiser>
Component: coreutilsAssignee: Ondrej Vasik <ovasik>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: kdudka, maxamillion, ooprala, ovasik, p, twaugh
Target Milestone: ---Keywords: FutureFeature, Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Linux   
Whiteboard:
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:
Cloudforms Team: ---

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