Details
-
Bug
-
Resolution: Fixed
-
Minor
-
None
-
None
-
3
-
9223372036854775807
Description
$ mkfifo myfifo $ lfs find myfifo myfifo $ lfs find myfifo --printf "%p\n" ** command hangs ** ^C $ sudo mknod mychardev c 1 5 $ lfs find mychardev mychardev $ lfs find mychardev --printf "%p\n" lfs: failed for 'mychardev': Inappropriate ioctl for device
Attachments
Issue Links
Activity
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50916/
Subject: LU-16808 lfs: return invalid=-1 if no projid retrieved
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 8d09b3e77744973d10d2029028d7e6f14591bc6d
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50905/
Subject: LU-16808 lfs: lfs find --printf won't hang on special files
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f47f816a5b1deabc1d501332a1a30af205d22515
Getting the projid on the symlink target is intentional behavior (LU-11872).
I noticed another (potential) issue with symlinks and the project ID. For most attributes it appears lfs find uses data from the symlink itself, but for project ID, lfs find seems to use the value from the target:
$ touch file20 && lfs project -p 20 file20 $ ln -s file20 link21 && lfs project -p 21 link21 $ lfs project . 20 - ./file20 21 - ./link21 # including %s in the format string to demonstrate that lfs find uses values from the link itself (and not the target) for attributes other than project id $ lfs find . --printf "%p %s %LP\n" . 4096 0 ./file20 0 20 ./link21 6 20
Would we expect that lfs find would use the value from the link in this case?
Interestingly, I ran a quick test on ext4 and found that (at least in my setup) projid on symlinks did not work:
ext4-mount $ touch file20 && chattr -p 20 file20
ext4-mount $ ln -s file20 link21
ext4-mount $ chattr -p 21 link21
chattr: Operation not supported while reading flags on link21
ext4-mount $ lsattr -p .
0 --------------e------- ./lost+found
lsattr: Operation not supported While reading flags on ./link21
20 --------------e------- ./file20
I am not sure what the "expected" behavior of projid and symlinks is so I don't know if the lfs find behavior above is problematic.
@adilger any thoughts on this? If the current behavior is wrong should I submit a ticket for this?
"Zhenyu Xu <bobijam@hotmail.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50916
Subject: LU-16808 lfs: return error if projid cannot be retrieved
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 37871eccbaafe1e2b7a07dd74070ef7ba2d40751
"Zhenyu Xu <bobijam@hotmail.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50905
Subject: LU-16808 lfs: lfs find --printf won't hang on special files
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 935a7716d744d2c2009a363f0b1bdbaee84d0895
The FIFO hang is probably because the open() immediately gets blocked on waiting for some data to be available on the pipe? The strace shows:
write(1, "/mnt/testfs/newdir\n", 19/mnt/testfs/newdir
) = 19
close(3) = 0
fstat(4, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
fcntl(4, F_GETFL) = 0x28800 (flags O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_NOFOLLOW)
fcntl(4, F_SETFD, FD_CLOEXEC) = 0
getdents(4, /* 3 entries */, 32768) = 72
ioctl(4, _IOC(_IOC_READ|_IOC_WRITE, 0x69, 0x16, 0x140), 0x16f3040) = 0
ioctl(4, _IOC(_IOC_READ, 0x66, 0xaf, 0x4), 0x7ffd455d7480) = 0
open("/mnt/testfs/newdir/char", O_RDONLY) = 3
ioctl(3, FS_IOC_FSGETXATTR, 0x7ffd455d70b0) = -1 ENOTTY (Inappropriate ioctl for device)
close(3) = 0
ioctl(4, _IOC(_IOC_READ|_IOC_WRITE, 0x69, 0x16, 0x140), 0x1f84040) = 0
ioctl(4, _IOC(_IOC_READ, 0x66, 0xaf, 0x4), 0x7ffc5a629f30) = 0
open("/mnt/testfs/newdir/fifo", O_RDONLY
I suspect that the "semantic traverse" can already use the file type in the directory entry to skip trying to process such files.
In fact, looking at the strace output for regular file processing it seems like it is doing far too much for most files already:
1683657120.708615 getdents(4, /* 102 entries */, 32768) = 6448 <0.000784>
1683657120.709513 ioctl(4, _IOC(_IOC_READ|_IOC_WRITE, 0x69, 0x16, 0x140), 0xea2040) = 0 <0.002730>
1683657120.712309 open("/mnt/testfs/dir/filename_is_long-15", O_RDONLY) = 3 <0.003517>
1683657120.715904 ioctl(3, _IOC(_IOC_READ, 0x66, 0xaf, 0x4), 0x7ffc0a9b3ba0) = 0 <0.000140>
1683657120.716106 ioctl(3, FS_IOC_FSGETXATTR, 0x7ffc0a9b37d0) = 0 <0.000044>
1683657120.716243 newfstatat(4, "filename_is_long-15", {st_mode=S_IFREG|0644, st_size=0, ...}, AT_SYMLINK_NOFOLLOW) = 0 <0.000683>
1683657120.717022 write(1, "/mnt/testfs/dir/filename_is_long-15", 36) = 36 <0.000296> /mnt/testfs/dir/filename_is_long-15
1683657120.717386 close(3) = 0 <0.002073>
The ioctl(4, _IOC(_IOC_READ|_IOC_WRITE, 0x69, 0x16, 0x140) = IOC_MDC_GETFILEINFO_V2) is expected for every file, since it gets the file layout and MDT attributes from the parent directory but we definitely should not be doing open() and ioctl(FS_IOC_FSGETXATTR) and newfstatat() on every file when we are only printing the pathname. That is a lot of overhead (more than double the original ioctl(IOC_MDC_GETFILEINFO_V2) call.
It looks like this was added as part of the -printf functionality, since running without -printf "%p" shows none of that overhead:
1683657295.265889 getdents(4, /* 102 entries */, 32768) = 6448 <0.000393> 1683657295.266362 write(1, "/mnt/testfs/dir/filename_is_long-15"..., 36) = 36 <0.000009> /mnt/testfs/dir/filename_is_long-15 1683657295.266446 write(1, "/mnt/testfs/dir/filename_is_long-44"..., 36) = 36 <0.000009> /mnt/testfs/dir/filename_is_long-44
I guess that is already described in LU-15837 under "selective fetching of metadata attributes. In particular, fetching the projid for a file requires an extra syscall that is unnecessary if the projid is not being printed" but it isn't just one syscall but multiple.
Also, I didn't look at why find hangs on fifos, that's probably the most problematic issue here.
Oh, and absolutely about the error message including the file name, that's obviously right. That could be a quick separate patch if you wanted to do it.
Landed for 2.16