Linux 4.11 compat: vfs_getattr() takes 4 args #608

ofaaland · 2017-03-07T22:40:26Z

There are changes to vfs_getattr() in torvalds/linux@a528d35. The new
interface is:

int vfs_getattr(const struct path *path, struct kstat *stat,
u32 request_mask, unsigned int query_flags)

The request_mask argument indicates which field(s) the caller intends to
use. Fields the caller does not specify via request_mask may be
returned anyway, but may be approximate.

The query_flags argument indicates whether the filesystem must update
the attributes from the backing store.

This patch uses the query_flags which result in vfs_getattr behaving the same
as it did with the 2-argument version which the kernel provided before
Linux 4.11.

Members blksize and blocks are now always the same size regardless of
arch. They match the size of the equivalent members in vnode_t.

The configure checks are modified to ensure that the appropriate
vfs_getattr() interface is used.

A more complete fix, removing the ZFS dependency on vfs_getattr()
entirely, is deferred as it is a much larger project.

Signed-off-by: Olaf Faaland [email protected]

tuxoko · 2017-03-08T00:04:34Z

module/spl/spl-vnode.c

@@ -153,7 +153,9 @@ vn_open(const char *path, uio_seg_t seg, int flags, int mode,
 	if (IS_ERR(fp))
 		return (-PTR_ERR(fp));

-#ifdef HAVE_2ARGS_VFS_GETATTR
+#if defined(HAVE_4ARGS_VFS_GETATTR)
+	rc = vfs_getattr(&fp->f_path, &stat, STATX_MODE, AT_STATX_SYNC_AS_STAT);


#define STATX_TYPE 0x00000001U /* Want/got stx_mode & S_IFMT */ #define STATX_MODE 0x00000002U /* Want/got stx_mode & ~S_IFMT */

I think you use the wrong one.

You're right, thanks for catching that.

ofaaland · 2017-03-08T00:49:06Z

A question about the configure checks for the vfs_getattr() interface change.
As I structured it here, I put the newest interface test at the top - checking for 4 arguments first. I did that because it makes it easy to remove the old one when the time comes, and hints at which interface is the newest.

However, it makes the diff quite a bit larger. I could instead just add the 4-arg test in the deepest fail case.

Do you have an opinion?

tuxoko · 2017-03-08T00:52:01Z

I think we should split each test as a function and do cascade call. Something like this openzfs/zfs@7a78934

It would be easier to read, and we don't have to keep indenting it. It would also diff well if we change them.

ofaaland · 2017-03-16T22:42:09Z

@tuxoko , there have been some changes to the SPL Linux 4.11 compat code since your approval of the PR. Please look again when you get a chance, thanks.

tuxoko · 2017-03-16T22:54:36Z

module/spl/spl-vnode.c

 	rc = vfs_getattr(&fp->f_path, &stat);
-#else
+#elif defined(HAVE_3ARGS_VFS_GETATTR)
 	rc = vfs_getattr(fp->f_path.mnt, fp->f_dentry, &stat);
 #endif


If would be better to use #else for the last one so that you don't end up with nothing here. Or add a #else #error

There are changes to vfs_getattr() in torvalds/linux@a528d35. The new interface is: int vfs_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int query_flags) The request_mask argument indicates which field(s) the caller intends to use. Fields the caller does not specify via request_mask may be set in the returned struct anyway, but their values may be approximate. The query_flags argument indicates whether the filesystem must update the attributes from the backing store. This patch uses the query_flags which result in vfs_getattr behaving the same as it did with the 2-argument version which the kernel provided before Linux 4.11. Members blksize and blocks are now always the same size regardless of arch. They match the size of the equivalent members in vnode_t. The configure checks are modified to ensure that the appropriate vfs_getattr() interface is used. A more complete fix, removing the ZFS dependency on vfs_getattr() entirely, is deferred as it is a much larger project. Signed-off-by: Olaf Faaland <[email protected]>

In Linux 4.11, torvalds/linux@2a1f062, signal handling related functions were moved from sched.h into sched/signal.h. Add configure checks to detect this and include the new file where needed. Signed-off-by: Olaf Faaland <[email protected]>

Before kernel 2.6.29 credentials were embedded in task_structs, and zfs had cases where one thread would need to refer to the credential of another thread, forcing it to take a hold on the foreign thread's task_struct to ensure it was not freed. Since 2.6.29, the credential has been moved out of the task_struct into a cred_t. In addition, the mainline kernel originally did not export __put_task_struct() but the RHEL5 kernel did, according to openzfs/spl@e811949a570. As of 2.6.39 the mainline kernel exports it. There is no longer zfs code that takes or releases holds on a task_struct, and so there is no longer any reference to __put_task_struct(). This affects the linux 4.11 kernel because the prototype for __put_task_struct() is in a new include file (linux/sched/task.h) and so the config check failed to detect the exported symbol. Removing the unnecessary stub and corresponding config check. This works on kernels since the oldest one currently supported, 2.6.32 as shipped with Centos/RHEL. Signed-off-by: Olaf Faaland <[email protected]>

Made requested changes.

In Linux 4.11, torvalds/linux@2a1f062, signal handling related functions were moved from sched.h into sched/signal.h. Add configure checks to detect this and include the new file where needed. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes #608

Before kernel 2.6.29 credentials were embedded in task_structs, and zfs had cases where one thread would need to refer to the credential of another thread, forcing it to take a hold on the foreign thread's task_struct to ensure it was not freed. Since 2.6.29, the credential has been moved out of the task_struct into a cred_t. In addition, the mainline kernel originally did not export __put_task_struct() but the RHEL5 kernel did, according to e811949a570. As of 2.6.39 the mainline kernel exports it. There is no longer zfs code that takes or releases holds on a task_struct, and so there is no longer any reference to __put_task_struct(). This affects the linux 4.11 kernel because the prototype for __put_task_struct() is in a new include file (linux/sched/task.h) and so the config check failed to detect the exported symbol. Removing the unnecessary stub and corresponding config check. This works on kernels since the oldest one currently supported, 2.6.32 as shipped with Centos/RHEL. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes #608

There are changes to vfs_getattr() in torvalds/linux@a528d35. The new interface is: int vfs_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int query_flags) The request_mask argument indicates which field(s) the caller intends to use. Fields the caller does not specify via request_mask may be set in the returned struct anyway, but their values may be approximate. The query_flags argument indicates whether the filesystem must update the attributes from the backing store. This patch uses the query_flags which result in vfs_getattr behaving the same as it did with the 2-argument version which the kernel provided before Linux 4.11. Members blksize and blocks are now always the same size regardless of arch. They match the size of the equivalent members in vnode_t. The configure checks are modified to ensure that the appropriate vfs_getattr() interface is used. A more complete fix, removing the ZFS dependency on vfs_getattr() entirely, is deferred as it is a much larger project. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes openzfs#608

In Linux 4.11, torvalds/linux@2a1f062, signal handling related functions were moved from sched.h into sched/signal.h. Add configure checks to detect this and include the new file where needed. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes openzfs#608

Before kernel 2.6.29 credentials were embedded in task_structs, and zfs had cases where one thread would need to refer to the credential of another thread, forcing it to take a hold on the foreign thread's task_struct to ensure it was not freed. Since 2.6.29, the credential has been moved out of the task_struct into a cred_t. In addition, the mainline kernel originally did not export __put_task_struct() but the RHEL5 kernel did, according to openzfs/spl@e811949a570. As of 2.6.39 the mainline kernel exports it. There is no longer zfs code that takes or releases holds on a task_struct, and so there is no longer any reference to __put_task_struct(). This affects the linux 4.11 kernel because the prototype for __put_task_struct() is in a new include file (linux/sched/task.h) and so the config check failed to detect the exported symbol. Removing the unnecessary stub and corresponding config check. This works on kernels since the oldest one currently supported, 2.6.32 as shipped with Centos/RHEL. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes openzfs#608

There are changes to vfs_getattr() in torvalds/linux@a528d35. The new interface is: int vfs_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int query_flags) The request_mask argument indicates which field(s) the caller intends to use. Fields the caller does not specify via request_mask may be set in the returned struct anyway, but their values may be approximate. The query_flags argument indicates whether the filesystem must update the attributes from the backing store. This patch uses the query_flags which result in vfs_getattr behaving the same as it did with the 2-argument version which the kernel provided before Linux 4.11. Members blksize and blocks are now always the same size regardless of arch. They match the size of the equivalent members in vnode_t. The configure checks are modified to ensure that the appropriate vfs_getattr() interface is used. A more complete fix, removing the ZFS dependency on vfs_getattr() entirely, is deferred as it is a much larger project. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes #608

In Linux 4.11, torvalds/linux@2a1f062, signal handling related functions were moved from sched.h into sched/signal.h. Add configure checks to detect this and include the new file where needed. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes #608

Before kernel 2.6.29 credentials were embedded in task_structs, and zfs had cases where one thread would need to refer to the credential of another thread, forcing it to take a hold on the foreign thread's task_struct to ensure it was not freed. Since 2.6.29, the credential has been moved out of the task_struct into a cred_t. In addition, the mainline kernel originally did not export __put_task_struct() but the RHEL5 kernel did, according to e811949a570. As of 2.6.39 the mainline kernel exports it. There is no longer zfs code that takes or releases holds on a task_struct, and so there is no longer any reference to __put_task_struct(). This affects the linux 4.11 kernel because the prototype for __put_task_struct() is in a new include file (linux/sched/task.h) and so the config check failed to detect the exported symbol. Removing the unnecessary stub and corresponding config check. This works on kernels since the oldest one currently supported, 2.6.32 as shipped with Centos/RHEL. Reviewed-by: Chunwei Chen <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Olaf Faaland <[email protected]> Closes #608

ofaaland added the Build Issue label Mar 7, 2017

ofaaland force-pushed the b_vfs_getattr branch from b5c341a to f63daf1 Compare March 7, 2017 22:53

tuxoko suggested changes Mar 8, 2017

View reviewed changes

ofaaland force-pushed the b_vfs_getattr branch 2 times, most recently from acada65 to 21bb330 Compare March 8, 2017 00:44

tuxoko approved these changes Mar 8, 2017

View reviewed changes

ofaaland force-pushed the b_vfs_getattr branch 4 times, most recently from eff53ab to 5a357dd Compare March 14, 2017 20:40

ofaaland force-pushed the b_vfs_getattr branch from 5a357dd to c1b2c8d Compare March 16, 2017 02:05

tuxoko previously requested changes Mar 16, 2017

View reviewed changes

ofaaland added 3 commits March 16, 2017 16:18

Linux 4.11 compat: add linux/sched/signal.h

a85810d

In Linux 4.11, torvalds/linux@2a1f062, signal handling related functions were moved from sched.h into sched/signal.h. Add configure checks to detect this and include the new file where needed. Signed-off-by: Olaf Faaland <[email protected]>

ofaaland force-pushed the b_vfs_getattr branch from c1b2c8d to 9d616cc Compare March 16, 2017 23:20

tuxoko approved these changes Mar 16, 2017

View reviewed changes

behlendorf approved these changes Mar 20, 2017

View reviewed changes

behlendorf closed this in 94b1ab2 Mar 21, 2017

ofaaland deleted the b_vfs_getattr branch September 19, 2017 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux 4.11 compat: vfs_getattr() takes 4 args #608

Linux 4.11 compat: vfs_getattr() takes 4 args #608

ofaaland commented Mar 7, 2017

tuxoko Mar 8, 2017

ofaaland Mar 8, 2017

ofaaland commented Mar 8, 2017

tuxoko commented Mar 8, 2017

ofaaland commented Mar 16, 2017 •

edited

Loading

tuxoko Mar 16, 2017

ofaaland Mar 16, 2017

Linux 4.11 compat: vfs_getattr() takes 4 args #608

Linux 4.11 compat: vfs_getattr() takes 4 args #608

Conversation

ofaaland commented Mar 7, 2017

tuxoko Mar 8, 2017

Choose a reason for hiding this comment

ofaaland Mar 8, 2017

Choose a reason for hiding this comment

ofaaland commented Mar 8, 2017

tuxoko commented Mar 8, 2017

ofaaland commented Mar 16, 2017 • edited Loading

tuxoko Mar 16, 2017

Choose a reason for hiding this comment

ofaaland Mar 16, 2017

Choose a reason for hiding this comment

ofaaland commented Mar 16, 2017 •

edited

Loading