| Message ID | 20240918-statx-stable-linux-6-10-y-v1-0-8364a071074f@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Backport statx(..., NULL, AT_EMPTY_PATH, ...) | expand |
On Wed, Sep 18, 2024 at 10:01:18PM +0800, Miao Wang via B4 Relay wrote: > Commit 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, > ...)") added support for passing in NULL when AT_EMPTY_PATH is given, > improving performance when statx is used for fetching stat informantion > from a given fd, which is especially important for 32-bit platforms. > This commit also improved the performance when an empty string is given > by short-circuiting the handling of such paths. > > This series is based on the commits in the Linus’ tree. Sligth > modifications are applied to the context of the patches for cleanly > applying. > > Tested-by: Xi Ruoyao <xry111@xry111.site> > Signed-off-by: Miao Wang <shankerwangmiao@gmail.com> This really looks like a brand new feature wanting to be backported, so why does it qualify under the stable kernel rules as fixing something? I am willing to take some kinds of "fixes performance issues" new features when the subsystem maintainers agree and ask for it, but that doesn't seem to be the case here, and so without their approval and agreement that this is relevant, we can't accept them. thanks, greg k-h
On Wed, 2024-09-18 at 19:33 +0200, Greg Kroah-Hartman wrote: > On Wed, Sep 18, 2024 at 10:01:18PM +0800, Miao Wang via B4 Relay wrote: > > Commit 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, > > ...)") added support for passing in NULL when AT_EMPTY_PATH is given, > > improving performance when statx is used for fetching stat informantion > > from a given fd, which is especially important for 32-bit platforms. > > This commit also improved the performance when an empty string is given > > by short-circuiting the handling of such paths. > > > > This series is based on the commits in the Linus’ tree. Sligth > > modifications are applied to the context of the patches for cleanly > > applying. > > > > Tested-by: Xi Ruoyao <xry111@xry111.site> > > Signed-off-by: Miao Wang <shankerwangmiao@gmail.com> > > This really looks like a brand new feature wanting to be backported, so > why does it qualify under the stable kernel rules as fixing something? > > I am willing to take some kinds of "fixes performance issues" new > features when the subsystem maintainers agree and ask for it, but that > doesn't seem to be the case here, and so without their approval and > agreement that this is relevant, we can't accept them. Unfortunately the performance issue fix and the new feature are in the same commit. Is it acceptable to separate out the performance fix part for stable? (Basically remove "if (!path) return true;" from the 1st patch.)
On Thu, Sep 19, 2024 at 01:37:17AM +0800, Xi Ruoyao wrote: > On Wed, 2024-09-18 at 19:33 +0200, Greg Kroah-Hartman wrote: > > On Wed, Sep 18, 2024 at 10:01:18PM +0800, Miao Wang via B4 Relay wrote: > > > Commit 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, > > > ...)") added support for passing in NULL when AT_EMPTY_PATH is given, > > > improving performance when statx is used for fetching stat informantion > > > from a given fd, which is especially important for 32-bit platforms. > > > This commit also improved the performance when an empty string is given > > > by short-circuiting the handling of such paths. > > > > > > This series is based on the commits in the Linus’ tree. Sligth > > > modifications are applied to the context of the patches for cleanly > > > applying. > > > > > > Tested-by: Xi Ruoyao <xry111@xry111.site> > > > Signed-off-by: Miao Wang <shankerwangmiao@gmail.com> > > > > This really looks like a brand new feature wanting to be backported, so > > why does it qualify under the stable kernel rules as fixing something? > > > > I am willing to take some kinds of "fixes performance issues" new > > features when the subsystem maintainers agree and ask for it, but that > > doesn't seem to be the case here, and so without their approval and > > agreement that this is relevant, we can't accept them. > > Unfortunately the performance issue fix and the new feature are in the > same commit. Is it acceptable to separate out the performance fix part > for stable? (Basically remove "if (!path) return true;" from the 1st > patch.) What prevents you, if you wish to have the increased performance, from just moving to a newer kernel version? We add new features and improvements like this all the time, why is this one so special to warrant doing backports. Especially with no maintainer or subsystem developer asking for this to be done? thanks, greg k-h
> 2024年9月19日 19:18，Greg Kroah-Hartman <gregkh@linuxfoundation.org> 写道： > > On Thu, Sep 19, 2024 at 01:37:17AM +0800, Xi Ruoyao wrote: >> On Wed, 2024-09-18 at 19:33 +0200, Greg Kroah-Hartman wrote: >>> On Wed, Sep 18, 2024 at 10:01:18PM +0800, Miao Wang via B4 Relay wrote: >>>> Commit 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, >>>> ...)") added support for passing in NULL when AT_EMPTY_PATH is given, >>>> improving performance when statx is used for fetching stat informantion >>>> from a given fd, which is especially important for 32-bit platforms. >>>> This commit also improved the performance when an empty string is given >>>> by short-circuiting the handling of such paths. >>>> >>>> This series is based on the commits in the Linus’ tree. Sligth >>>> modifications are applied to the context of the patches for cleanly >>>> applying. >>>> >>>> Tested-by: Xi Ruoyao <xry111@xry111.site> >>>> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com> >>> >>> This really looks like a brand new feature wanting to be backported, so >>> why does it qualify under the stable kernel rules as fixing something? >>> >>> I am willing to take some kinds of "fixes performance issues" new >>> features when the subsystem maintainers agree and ask for it, but that >>> doesn't seem to be the case here, and so without their approval and >>> agreement that this is relevant, we can't accept them. >> >> Unfortunately the performance issue fix and the new feature are in the >> same commit. Is it acceptable to separate out the performance fix part >> for stable? (Basically remove "if (!path) return true;" from the 1st >> patch.) > > What prevents you, if you wish to have the increased performance, from > just moving to a newer kernel version? We add new features and > improvements like this all the time, why is this one so special to > warrant doing backports. Especially with no maintainer or subsystem > developer asking for this to be done? We all know the long process from a new improvement getting into the mainline kernel to landing in users' devices. Considering 32-bit archectures which lacks 64-bit time support in fstat(), statx(fd, AT_EMPTY_PATH) is heavily relied on. My intention on putting up this backport is that to quicken this process, benefiting these users. Another reason is about loongarch. fstat() was not included in loongarch initially, until 6.11. Although the re-inclusion of fstat() is backported to stable releases, we still have implementation problems on the glibc side, that loongarch is the only architecture that may lack the support of fstat. If dynamic probing of the existence of fstat() is implemented, this code path would be only effective on loongarch, adding another layer of mess to the current fstat implementation and adding maintenance burden to glibc maintainers. Instead, if we choose to utilize statx(fd, NULL, AT_EMPTY_PATH), even if using dynamic probing, loongarch and all other 32-bit architectures using statx() for fstat() can benefit from this. Based on the same reason why you have accepted the re-inclusion of fstat on loongarch into the stable trees, I believe it would be potentially possible to let the statx(..., NULL, AT_EMPTY_PATH, ...) get into the stable trees as well. Thanks again for your considering this. Since VFS maintainers are being looped in the thread, if their approval is necessary, I'm happy to listen to their opinions. Cheers, Miao Wang
Commit 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, ...)") added support for passing in NULL when AT_EMPTY_PATH is given, improving performance when statx is used for fetching stat informantion from a given fd, which is especially important for 32-bit platforms. This commit also improved the performance when an empty string is given by short-circuiting the handling of such paths. This series is based on the commits in the Linus’ tree. Sligth modifications are applied to the context of the patches for cleanly applying. Tested-by: Xi Ruoyao <xry111@xry111.site> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com> --- Christian Brauner (2): fs: new helper vfs_empty_path() stat: use vfs_empty_path() helper Mateusz Guzik (1): vfs: support statx(..., NULL, AT_EMPTY_PATH, ...) fs/internal.h | 14 ++++++ fs/namespace.c | 13 ------ fs/stat.c | 123 ++++++++++++++++++++++++++++++++++++----------------- include/linux/fs.h | 17 ++++++++ 4 files changed, 116 insertions(+), 51 deletions(-) --- base-commit: 049be94099ea5ba8338526c5a4f4f404b9dcaf54 change-id: 20240918-statx-stable-linux-6-10-y-17c3ec7497c8 Best regards,