Discussion:
Why does readdir() open files ?
Ben RUBSON
2018-03-28 16:00:19 UTC
Permalink
Hi,

I'm porting a FUSE FS to Cygwin, and I focus especially on readdir()
performance.

What I see is that when readdir() occurs, each file of the directory is as
expected (at least I think) stated (getattr), but also opened (open).
The first block of the file was even red, until I set the notexec mount
option.

My question is, why is every file opened ?
This is quite a performance killer, particularly for a FUSE FS.

I thought this was to calculate the inode number, I then set the ihash
mount option, but it did not help.

Thank you very much,

Best regards,

Ben
Ben RUBSON
2018-03-28 16:35:55 UTC
Permalink
Post by Ben RUBSON
Hi,
I'm porting a FUSE FS to Cygwin, and I focus especially on readdir()
performance.
What I see is that when readdir() occurs, each file of the directory is
as expected (at least I think) stated (getattr), but also opened (open).
The first block of the file was even red, until I set the notexec mount
option.
My question is, why is every file opened ?
This is quite a performance killer, particularly for a FUSE FS.
I thought this was to calculate the inode number, I then set the ihash
mount option, but it did not help.
I may be totally wrong, but perhaps the culprit is the following code path ?
https://github.com/mirror/newlib-cygwin/blob/master/winsup/cygwin/fhandler_disk_file.cc#L2262
Perhaps we simply don't need to open the file if (!hasgood_inode ()) ?

That would be a nice performance improvement !

Thank you again,

Ben
Corinna Vinschen
2018-03-28 17:03:00 UTC
Permalink
Post by Ben RUBSON
Post by Ben RUBSON
Hi,
I'm porting a FUSE FS to Cygwin, and I focus especially on readdir()
performance.
What I see is that when readdir() occurs, each file of the directory is
as expected (at least I think) stated (getattr), but also opened (open).
The first block of the file was even red, until I set the notexec mount
option.
My question is, why is every file opened ?
This is quite a performance killer, particularly for a FUSE FS.
I thought this was to calculate the inode number, I then set the ihash
mount option, but it did not help.
I may be totally wrong, but perhaps the culprit is the following code path ?
https://github.com/mirror/newlib-cygwin/blob/master/winsup/cygwin/fhandler_disk_file.cc#L2262
Perhaps we simply don't need to open the file if (!hasgood_inode ()) ?
That would be a nice performance improvement !
You should make sure that the hasgood_inode() check works for your FUSE
FS. Check at the end of fs_info::update():

has_acls (flags () & FS_PERSISTENT_ACLS);
/* Netapp inode numbers are fly-by-night. */
hasgood_inode ((has_acls () && !is_netapp ()) || is_nfs ());

So your filesystem should return the FS_PERSISTENT_ACLS flags in a call
to NtQueryVolumeInformationFile(..., FileFsAttributeInformation)


Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
Ben RUBSON
2018-03-28 17:51:06 UTC
Permalink
Post by Corinna Vinschen
Post by Ben RUBSON
Post by Ben RUBSON
Hi,
I'm porting a FUSE FS to Cygwin, and I focus especially on readdir()
performance.
What I see is that when readdir() occurs, each file of the directory is
as expected (at least I think) stated (getattr), but also opened (open).
The first block of the file was even red, until I set the notexec mount
option.
My question is, why is every file opened ?
This is quite a performance killer, particularly for a FUSE FS.
I thought this was to calculate the inode number, I then set the ihash
mount option, but it did not help.
I may be totally wrong, but perhaps the culprit is the following code path ?
https://github.com/mirror/newlib-cygwin/blob/master/winsup/cygwin/fhandler_disk_file.cc#L2262
Perhaps we simply don't need to open the file if (!hasgood_inode ()) ?
That would be a nice performance improvement !
You should make sure that the hasgood_inode() check works for your FUSE
has_acls (flags () & FS_PERSISTENT_ACLS);
/* Netapp inode numbers are fly-by-night. */
hasgood_inode ((has_acls () && !is_netapp ()) || is_nfs ());
So your filesystem should return the FS_PERSISTENT_ACLS flags in a call
to NtQueryVolumeInformationFile(..., FileFsAttributeInformation)
Thank you for your answer Corinna.

You assume FS_PERSISTENT_ACLS must be set, otherwise I would not pass the
following dir->__flags condition ?
if (de->d_ino == 0 && (dir->__flags & dirent_set_d_ino))
https://github.com/mirror/newlib-cygwin/blob/master/winsup/cygwin/fhandler_disk_file.cc#L2225

So, as goal is to avoid opening files, I then should remove this flag from
my FS ?

I found that hasgood_inode() is also defined in winsup/cygwin/path.h :
bool hasgood_inode () const {return !(path_flags & PATH_IHASH); }

And when I set the ihash mount option, it is correctly honoured : generated
inode numbers differ from the "true" ones.
Can't we also rely on the ihash mount option to avoid opening files ?

Thank you again for your clarification & support !

Ben
Corinna Vinschen
2018-03-28 19:03:00 UTC
Permalink
Post by Ben RUBSON
Post by Corinna Vinschen
Post by Ben RUBSON
Post by Ben RUBSON
Hi,
I'm porting a FUSE FS to Cygwin, and I focus especially on readdir()
performance.
What I see is that when readdir() occurs, each file of the directory is
as expected (at least I think) stated (getattr), but also opened (open).
The first block of the file was even red, until I set the notexec mount
option.
My question is, why is every file opened ?
This is quite a performance killer, particularly for a FUSE FS.
I thought this was to calculate the inode number, I then set the ihash
mount option, but it did not help.
I may be totally wrong, but perhaps the culprit is the following code path ?
https://github.com/mirror/newlib-cygwin/blob/master/winsup/cygwin/fhandler_disk_file.cc#L2262
Perhaps we simply don't need to open the file if (!hasgood_inode ()) ?
That would be a nice performance improvement !
You should make sure that the hasgood_inode() check works for your FUSE
has_acls (flags () & FS_PERSISTENT_ACLS);
/* Netapp inode numbers are fly-by-night. */
hasgood_inode ((has_acls () && !is_netapp ()) || is_nfs ());
So your filesystem should return the FS_PERSISTENT_ACLS flags in a call
to NtQueryVolumeInformationFile(..., FileFsAttributeInformation)
Thank you for your answer Corinna.
You assume FS_PERSISTENT_ACLS must be set, otherwise I would not pass the
following dir->__flags condition ?
if (de->d_ino == 0 && (dir->__flags & dirent_set_d_ino))
https://github.com/mirror/newlib-cygwin/blob/master/winsup/cygwin/fhandler_disk_file.cc#L2225
So, as goal is to avoid opening files, I then should remove this flag from
my FS ?
What? No! This is not what I was talking about.

- Make sure the filesystem returns valid inodes and make sure
to set FS_PERSISTENT_ACLS.

- Also make sure that NtQueryDirectoryFile(FileIdBothDirectoryInformation)
works on your FS, and make sure to return a valid inode number in the
structure.

This is the best option you have. Everything else leads to workarounds
in Cygwin to make the results as POSIXy as possible.
Post by Ben RUBSON
bool hasgood_inode () const {return !(path_flags & PATH_IHASH); }
And when I set the ihash mount option,
Don't do that. The ihash option is a workaround for inferior filesystems.
It's set by the path_conv routine if hasgood_inode fails, or the user
can set the ihash flag for a Cygwin mount point.


Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
Loading...