Re: [PATCH 0/3] Backport statx(..., NULL, AT_EMPTY_PATH, ...)

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Sep 19, 2024 at 08:18:10PM +0800, Miao Wang wrote:
> 
> > 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.

That was a simple patch that only affected one arch, unlike this patch
series which touches core kernel code used by everyone.

Please just encourage users of this hardware to use a newer kernel if
they wish to take advantage of the performance increase.

thanks,

greg k-h