* Regression on linux-next (next-20250120)
@ 2025-01-23 15:41 Borah, Chaitanya Kumar
2025-01-23 18:18 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Borah, Chaitanya Kumar @ 2025-01-23 15:41 UTC (permalink / raw)
To: viro@zeniv.linux.org.uk
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Kurmi, Suresh Kumar, Saarinen, Jani,
linux-fsdevel@vger.kernel.org
Hello Al,
Hope you are doing well. I am Chaitanya from the linux graphics team in Intel.
This mail is regarding a regression we are seeing in our CI runs[1] on linux-next repository.
Since the version next-20250120 [2], we are seeing the following regression
`````````````````````````````````````````````````````````````````````````````````
<4>[ 21.520314] Oops: Oops: 0010 [#1] PREEMPT SMP NOPTI
<4>[ 21.520317] CPU: 11 UID: 0 PID: 1384 Comm: debugfs_test Not tainted 6.13.0-next-20250121-next-20250121-gf066b5a6c7a0+ #1
<4>[ 21.520319] Hardware name: ASUS System Product Name/PRIME Z790-P WIFI, BIOS 0812 02/24/2023
<4>[ 21.520320] RIP: 0010:0x0
<4>[ 21.520324] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
<4>[ 21.520325] RSP: 0018:ffffc90001f77b58 EFLAGS: 00010282
<4>[ 21.520327] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffc90001f77c48
<4>[ 21.520328] RDX: 0000000000000200 RSI: 00007fff2141fe40 RDI: ffff88810e38d440
<4>[ 21.520329] RBP: ffffc90001f77b98 R08: 0000000000000000 R09: 0000000000000000
<4>[ 21.520331] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881497ba648
<4>[ 21.520332] R13: ffff88810e38d440 R14: ffff88812fd5f400 R15: ffffc90001f77c48
<4>[ 21.520333] FS: 00007e64c0ee8900(0000) GS:ffff88885f180000(0000) knlGS:0000000000000000
<4>[ 21.520334] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 21.520336] CR2: ffffffffffffffd6 CR3: 0000000120674000 CR4: 0000000000f52ef0
<4>[ 21.520337] PKRU: 55555554
<4>[ 21.520338] Call Trace:
<4>[ 21.520339] <TASK>
<4>[ 21.520341] ? show_regs+0x6c/0x80
<4>[ 21.520345] ? __die+0x24/0x80
<4>[ 21.520347] ? page_fault_oops+0x175/0x5d0
<4>[ 21.520351] ? do_user_addr_fault+0x4c6/0x9d0
<4>[ 21.520354] ? exc_page_fault+0x8a/0x300
<4>[ 21.520357] ? asm_exc_page_fault+0x27/0x30
<4>[ 21.520362] full_proxy_read+0x6b/0xb0
<4>[ 21.520366] vfs_read+0xf9/0x390
.
`````````````````````````````````````````````````````````````````````````````````
Details log can be found in [3].
After bisecting the tree, the following patch [4] seems to be the first "bad" commit
`````````````````````````````````````````````````````````````````````````````````````````````````````````
commit 41a0ecc0997cd40d913cce18867efd1c34c64e28
Author: Al Viro mailto:viro@zeniv.linux.org.uk
Date: Sun Jan 12 08:06:47 2025 +0000
debugfs: get rid of dynamically allocation proxy_ops
`````````````````````````````````````````````````````````````````````````````````````````````````````````
We could not revert the patch but resetting to the parent of the commit seems to fix the issue
Could you please check why the patch causes this regression and provide a fix if necessary?
Thank you.
Regards
Chaitanya
[1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250120
[3] https://intel-gfx-ci.01.org/tree/linux-next/next-20250120/bat-rpls-4/dmesg0.txt
[4] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250120&id=41a0ecc0997cd40d913cce18867efd1c34c64e28
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-23 15:41 Regression on linux-next (next-20250120) Borah, Chaitanya Kumar
@ 2025-01-23 18:18 ` Al Viro
2025-01-24 13:55 ` Borah, Chaitanya Kumar
2025-01-26 15:54 ` Alexander Gordeev
0 siblings, 2 replies; 12+ messages in thread
From: Al Viro @ 2025-01-23 18:18 UTC (permalink / raw)
To: Borah, Chaitanya Kumar
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Kurmi, Suresh Kumar, Saarinen, Jani,
linux-fsdevel@vger.kernel.org
On Thu, Jan 23, 2025 at 03:41:08PM +0000, Borah, Chaitanya Kumar wrote:
> Hello Al,
>
> Hope you are doing well. I am Chaitanya from the linux graphics team in Intel.
>
> This mail is regarding a regression we are seeing in our CI runs[1] on linux-next repository.
>
> Since the version next-20250120 [2], we are seeing the following regression
Ugh... To narrow the things down, could you see if replacing
fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
with
fsd = kzalloc(sizeof(*fsd), GFP_KERNEL);
in fs/debugfs/file.c:__debugfs_file_get() affects the test?
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: Regression on linux-next (next-20250120)
2025-01-23 18:18 ` Al Viro
@ 2025-01-24 13:55 ` Borah, Chaitanya Kumar
2025-01-26 15:54 ` Alexander Gordeev
1 sibling, 0 replies; 12+ messages in thread
From: Borah, Chaitanya Kumar @ 2025-01-24 13:55 UTC (permalink / raw)
To: Al Viro
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Kurmi, Suresh Kumar, Saarinen, Jani,
linux-fsdevel@vger.kernel.org
> -----Original Message-----
> From: Al Viro <viro@ftp.linux.org.uk> On Behalf Of Al Viro
> Sent: Thursday, January 23, 2025 11:49 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Kurmi,
> Suresh Kumar <suresh.kumar.kurmi@intel.com>; Saarinen, Jani
> <jani.saarinen@intel.com>; linux-fsdevel@vger.kernel.org
> Subject: Re: Regression on linux-next (next-20250120)
>
> On Thu, Jan 23, 2025 at 03:41:08PM +0000, Borah, Chaitanya Kumar wrote:
> > Hello Al,
> >
> > Hope you are doing well. I am Chaitanya from the linux graphics team in
> Intel.
> >
> > This mail is regarding a regression we are seeing in our CI runs[1] on linux-
> next repository.
> >
> > Since the version next-20250120 [2], we are seeing the following
> > regression
>
> Ugh... To narrow the things down, could you see if replacing
> fsd = kmalloc(sizeof(*fsd), GFP_KERNEL); with
> fsd = kzalloc(sizeof(*fsd), GFP_KERNEL); in
> fs/debugfs/file.c:__debugfs_file_get() affects the test?
This change seems to help. Can we expect a fix patch?
Thank you.
Regards
Chaitanya
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-23 18:18 ` Al Viro
2025-01-24 13:55 ` Borah, Chaitanya Kumar
@ 2025-01-26 15:54 ` Alexander Gordeev
2025-01-27 5:04 ` Al Viro
1 sibling, 1 reply; 12+ messages in thread
From: Alexander Gordeev @ 2025-01-26 15:54 UTC (permalink / raw)
To: Al Viro
Cc: Borah, Chaitanya Kumar, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org
On Thu, Jan 23, 2025 at 06:18:53PM +0000, Al Viro wrote:
Hi Al,
> On Thu, Jan 23, 2025 at 03:41:08PM +0000, Borah, Chaitanya Kumar wrote:
> > Hello Al,
> >
> > Hope you are doing well. I am Chaitanya from the linux graphics team in Intel.
> >
> > This mail is regarding a regression we are seeing in our CI runs[1] on linux-next repository.
> >
> > Since the version next-20250120 [2], we are seeing the following regression
>
> Ugh... To narrow the things down, could you see if replacing
> fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
> with
> fsd = kzalloc(sizeof(*fsd), GFP_KERNEL);
> in fs/debugfs/file.c:__debugfs_file_get() affects the test?
This change fixes lots of the below failures in our CI. FWIW:
Tested-by: Alexander Gordeev <agordeev@linux.ibm.com>
Unable to handle kernel pointer dereference in virtual kernel address space
Failing address: 0000000000000000 TEID: 0000000000000000
Fault in primary space mode while using kernel ASCE.
AS:0000000243668007 R3:00000003fee58007 S:00000003fee57801 P:000000000000013d
Oops: 0004 ilc:1 [#1] SMP
Modules linked in: binfmt_misc mlx5_ib ib_uverbs ib_core dm_service_time nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables mlx5_core pkey_pckmo uvdevice s390_trng rng_core eadm_sch vfio_ccw mdev vfio_iommu_type1 vfio sch_fq_codel drm i2c_core loop dm_multipath drm_panel_orientation_quirks configfs nfnetlink lcs ctcm fsm zfcp scsi_transport_fc hmac_s390 ghash_s390 prng chacha_s390 libchacha aes_s390 des_s390 libdes sha3_512_s390 sha3_256_s390 sha512_s390 sha256_s390 sha1_s390 sha_common scsi_dh_rdac scsi_dh_emc scsi_dh_alua pkey autofs4 ecdsa_generic ecc
CPU: 3 UID: 0 PID: 19223 Comm: dump2tar Not tainted 6.14.0-20250123.rc0.git129.853d1f41ba73.300.fc41.s390x+next #1
Hardware name: IBM 9175 ME1 701 (LPAR)
Krnl PSW : 0704e00180000000 0000000000000000 (0x0)
R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
Krnl GPRS: 0000000000000003 0000029253f7f100 00000000a9fa0600 000003ff84472010
0000000000080000 00000212db803e68 00000212db803e68 0000000000080000
00000000cf33c6c0 00000000be6d6a00 00000000a9fa0600 0000000000000000
000003ff909acfa0 000003ff84472010 00000292d425da96 00000212db803c98
Krnl Code:>0000000000000000: 0000 illegal
0000000000000002: 0000 illegal
0000000000000004: 0000 illegal
0000000000000006: 0000 illegal
0000000000000008: 0000 illegal
000000000000000a: 0000 illegal
000000000000000c: 0000 illegal
000000000000000e: 0000 illegal
Call Trace:
[<0000000000000000>] 0x0
([<000001e12d421b6a>] full_proxy_read+0x4a/0xc0)
[<000001e12d18ac16>] vfs_read+0x96/0x340
[<000001e12d18b7b8>] ksys_read+0x78/0x100
[<000001e12cdbf9d6>] do_syscall.constprop.0+0x116/0x140
[<000001e12dba3ff4>] __do_syscall+0xd4/0x1c0
[<000001e12dbaf404>] system_call+0x74/0x98
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-26 15:54 ` Alexander Gordeev
@ 2025-01-27 5:04 ` Al Viro
2025-01-28 16:00 ` Borah, Chaitanya Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2025-01-27 5:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Borah, Chaitanya Kumar, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org, Alexander Gordeev
On Sun, Jan 26, 2025 at 04:54:55PM +0100, Alexander Gordeev wrote:
> > > Since the version next-20250120 [2], we are seeing the following regression
> >
> > Ugh... To narrow the things down, could you see if replacing
> > fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
> > with
> > fsd = kzalloc(sizeof(*fsd), GFP_KERNEL);
> > in fs/debugfs/file.c:__debugfs_file_get() affects the test?
>
> This change fixes lots of the below failures in our CI. FWIW:
>
> Tested-by: Alexander Gordeev <agordeev@linux.ibm.com>
The real fix follows:
[PATCH] Fix the missing initializations in __debugfs_file_get()
both method table pointers in debugfs_fsdata need to be initialized,
obviously...
Fixes: 41a0ecc0997c "debugfs: get rid of dynamically allocation proxy_ops"
Fucked-up-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index e33cc77699cd..212cd8128e1f 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -111,6 +111,7 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
fsd->methods |= HAS_READ;
if (ops->write)
fsd->methods |= HAS_WRITE;
+ fsd->real_fops = NULL;
} else {
const struct file_operations *ops;
ops = fsd->real_fops = DEBUGFS_I(inode)->real_fops;
@@ -124,6 +125,7 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
fsd->methods |= HAS_IOCTL;
if (ops->poll)
fsd->methods |= HAS_POLL;
+ fsd->short_fops = NULL;
}
refcount_set(&fsd->active_users, 1);
init_completion(&fsd->active_users_drained);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: Regression on linux-next (next-20250120)
2025-01-27 5:04 ` Al Viro
@ 2025-01-28 16:00 ` Borah, Chaitanya Kumar
2025-01-29 4:37 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Borah, Chaitanya Kumar @ 2025-01-28 16:00 UTC (permalink / raw)
To: Al Viro, Greg Kroah-Hartman
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Kurmi, Suresh Kumar, Saarinen, Jani,
linux-fsdevel@vger.kernel.org, Alexander Gordeev
Hello Al,
> -----Original Message-----
> From: Al Viro <viro@ftp.linux.org.uk> On Behalf Of Al Viro
> Sent: Monday, January 27, 2025 10:34 AM
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Kurmi, Suresh
> Kumar <suresh.kumar.kurmi@intel.com>; Saarinen, Jani
> <jani.saarinen@intel.com>; linux-fsdevel@vger.kernel.org; Alexander Gordeev
> <agordeev@linux.ibm.com>
> Subject: Re: Regression on linux-next (next-20250120)
>
> On Sun, Jan 26, 2025 at 04:54:55PM +0100, Alexander Gordeev wrote:
>
> > > > Since the version next-20250120 [2], we are seeing the following
> > > > regression
> > >
> > > Ugh... To narrow the things down, could you see if replacing
> > > fsd = kmalloc(sizeof(*fsd), GFP_KERNEL); with
> > > fsd = kzalloc(sizeof(*fsd), GFP_KERNEL); in
> > > fs/debugfs/file.c:__debugfs_file_get() affects the test?
> >
> > This change fixes lots of the below failures in our CI. FWIW:
> >
> > Tested-by: Alexander Gordeev <agordeev@linux.ibm.com>
>
> The real fix follows:
>
> [PATCH] Fix the missing initializations in __debugfs_file_get()
>
> both method table pointers in debugfs_fsdata need to be initialized,
> obviously...
>
> Fixes: 41a0ecc0997c "debugfs: get rid of dynamically allocation proxy_ops"
> Fucked-up-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index
> e33cc77699cd..212cd8128e1f 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -111,6 +111,7 @@ static int __debugfs_file_get(struct dentry *dentry,
> enum dbgfs_get_mode mode)
> fsd->methods |= HAS_READ;
> if (ops->write)
> fsd->methods |= HAS_WRITE;
> + fsd->real_fops = NULL;
> } else {
> const struct file_operations *ops;
> ops = fsd->real_fops = DEBUGFS_I(inode)->real_fops;
> @@ -124,6 +125,7 @@ static int __debugfs_file_get(struct dentry *dentry,
> enum dbgfs_get_mode mode)
> fsd->methods |= HAS_IOCTL;
> if (ops->poll)
> fsd->methods |= HAS_POLL;
> + fsd->short_fops = NULL;
> }
> refcount_set(&fsd->active_users, 1);
> init_completion(&fsd->active_users_drained);
Unfortunately this change does not help us. I think it is the methods member that causes the problem. So the following change solves the problem for us.
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -102,6 +102,8 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
if (!fsd)
return -ENOMEM;
+ fsd->methods = 0;
+
if (mode == DBGFS_GET_SHORT) {
const struct debugfs_short_fops *ops;
ops = fsd->short_fops = DEBUGFS_I(inode)->short_fops;
My guess is, since methods have some junk value in it, we are trying to call a read function for a debugfs entry for which it doesn't exist. That leads to the failure.
<4>[ 34.240163] Call Trace:
<4>[ 34.240164] <TASK>
<4>[ 34.240165] ? show_regs+0x6c/0x80
<4>[ 34.240169] ? __die+0x24/0x80
<4>[ 34.240171] ? page_fault_oops+0x175/0x5d0
<4>[ 34.240176] ? do_user_addr_fault+0x4c6/0x9d0
<4>[ 34.240179] ? exc_page_fault+0x8a/0x300
<4>[ 34.240182] ? asm_exc_page_fault+0x27/0x30
<4>[ 34.240187] *full_proxy_read*+0x6b/0xb0
<4>[ 34.240191] vfs_read+0xf9/0x390
My take would be to stick with the kzalloc like you suggested.
Regards
Chaitanya
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-28 16:00 ` Borah, Chaitanya Kumar
@ 2025-01-29 4:37 ` Al Viro
2025-01-29 7:13 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2025-01-29 4:37 UTC (permalink / raw)
To: Borah, Chaitanya Kumar
Cc: Greg Kroah-Hartman, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org, Alexander Gordeev
On Tue, Jan 28, 2025 at 04:00:58PM +0000, Borah, Chaitanya Kumar wrote:
> Unfortunately this change does not help us. I think it is the methods member that causes the problem. So the following change solves the problem for us.
>
>
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -102,6 +102,8 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
> if (!fsd)
> return -ENOMEM;
>
> + fsd->methods = 0;
> +
> if (mode == DBGFS_GET_SHORT) {
> const struct debugfs_short_fops *ops;
> ops = fsd->short_fops = DEBUGFS_I(inode)->short_fops;
D'OH.
Both are needed, actually. Slightly longer term I would rather
split full_proxy_{read,write,lseek}() into short and full variant,
getting rid of the "check which pointer is non-NULL" and killed
the two remaining users of debugfs_real_fops() outside of
fs/debugfs/file.c; then we could union these ->..._fops pointers,
but until then they need to be initialized.
And yes, ->methods obviously needs to be initialized.
Al, bloody embarrassed ;-/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-29 4:37 ` Al Viro
@ 2025-01-29 7:13 ` Greg Kroah-Hartman
2025-01-29 19:19 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2025-01-29 7:13 UTC (permalink / raw)
To: Al Viro
Cc: Borah, Chaitanya Kumar, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org, Alexander Gordeev
On Wed, Jan 29, 2025 at 04:37:12AM +0000, Al Viro wrote:
> On Tue, Jan 28, 2025 at 04:00:58PM +0000, Borah, Chaitanya Kumar wrote:
>
> > Unfortunately this change does not help us. I think it is the methods member that causes the problem. So the following change solves the problem for us.
> >
> >
> > --- a/fs/debugfs/file.c
> > +++ b/fs/debugfs/file.c
> > @@ -102,6 +102,8 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
> > if (!fsd)
> > return -ENOMEM;
> >
> > + fsd->methods = 0;
> > +
> > if (mode == DBGFS_GET_SHORT) {
> > const struct debugfs_short_fops *ops;
> > ops = fsd->short_fops = DEBUGFS_I(inode)->short_fops;
>
> D'OH.
>
> Both are needed, actually. Slightly longer term I would rather
> split full_proxy_{read,write,lseek}() into short and full variant,
> getting rid of the "check which pointer is non-NULL" and killed
> the two remaining users of debugfs_real_fops() outside of
> fs/debugfs/file.c; then we could union these ->..._fops pointers,
> but until then they need to be initialized.
>
> And yes, ->methods obviously needs to be initialized.
>
> Al, bloody embarrassed ;-/
No worries, want to send a patch to fix both of these up so we can fix
up Linus's tree now?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-29 7:13 ` Greg Kroah-Hartman
@ 2025-01-29 19:19 ` Al Viro
2025-01-30 7:25 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2025-01-29 19:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Borah, Chaitanya Kumar, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org, Alexander Gordeev
On Wed, Jan 29, 2025 at 08:13:02AM +0100, Greg Kroah-Hartman wrote:
> > Both are needed, actually. Slightly longer term I would rather
> > split full_proxy_{read,write,lseek}() into short and full variant,
> > getting rid of the "check which pointer is non-NULL" and killed
> > the two remaining users of debugfs_real_fops() outside of
> > fs/debugfs/file.c; then we could union these ->..._fops pointers,
> > but until then they need to be initialized.
> >
> > And yes, ->methods obviously needs to be initialized.
> >
> > Al, bloody embarrassed ;-/
>
> No worries, want to send a patch to fix both of these up so we can fix
> up Linus's tree now?
[PATCH] Fix the missing initializations in __debugfs_file_get()
both method table pointers in debugfs_fsdata need to be initialized,
obviously, and calculating the bitmap of present methods would also
go better if we start with initialized state.
Fixes: 41a0ecc0997c "debugfs: get rid of dynamically allocation proxy_ops"
Fucked-up-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index e33cc77699cd..69e9ddcb113d 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -94,6 +94,7 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
fsd = d_fsd;
} else {
struct inode *inode = dentry->d_inode;
+ unsigned int methods = 0;
if (WARN_ON(mode == DBGFS_GET_ALREADY))
return -EINVAL;
@@ -106,25 +107,28 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
const struct debugfs_short_fops *ops;
ops = fsd->short_fops = DEBUGFS_I(inode)->short_fops;
if (ops->llseek)
- fsd->methods |= HAS_LSEEK;
+ methods |= HAS_LSEEK;
if (ops->read)
- fsd->methods |= HAS_READ;
+ methods |= HAS_READ;
if (ops->write)
- fsd->methods |= HAS_WRITE;
+ methods |= HAS_WRITE;
+ fsd->real_fops = NULL;
} else {
const struct file_operations *ops;
ops = fsd->real_fops = DEBUGFS_I(inode)->real_fops;
if (ops->llseek)
- fsd->methods |= HAS_LSEEK;
+ methods |= HAS_LSEEK;
if (ops->read)
- fsd->methods |= HAS_READ;
+ methods |= HAS_READ;
if (ops->write)
- fsd->methods |= HAS_WRITE;
+ methods |= HAS_WRITE;
if (ops->unlocked_ioctl)
- fsd->methods |= HAS_IOCTL;
+ methods |= HAS_IOCTL;
if (ops->poll)
- fsd->methods |= HAS_POLL;
+ methods |= HAS_POLL;
+ fsd->short_fops = NULL;
}
+ fsd->methods = methods;
refcount_set(&fsd->active_users, 1);
init_completion(&fsd->active_users_drained);
INIT_LIST_HEAD(&fsd->cancellations);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-29 19:19 ` Al Viro
@ 2025-01-30 7:25 ` Greg Kroah-Hartman
2025-01-30 8:08 ` Borah, Chaitanya Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2025-01-30 7:25 UTC (permalink / raw)
To: Al Viro
Cc: Borah, Chaitanya Kumar, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org, Alexander Gordeev
On Wed, Jan 29, 2025 at 07:19:37PM +0000, Al Viro wrote:
> On Wed, Jan 29, 2025 at 08:13:02AM +0100, Greg Kroah-Hartman wrote:
>
> > > Both are needed, actually. Slightly longer term I would rather
> > > split full_proxy_{read,write,lseek}() into short and full variant,
> > > getting rid of the "check which pointer is non-NULL" and killed
> > > the two remaining users of debugfs_real_fops() outside of
> > > fs/debugfs/file.c; then we could union these ->..._fops pointers,
> > > but until then they need to be initialized.
> > >
> > > And yes, ->methods obviously needs to be initialized.
> > >
> > > Al, bloody embarrassed ;-/
> >
> > No worries, want to send a patch to fix both of these up so we can fix
> > up Linus's tree now?
>
> [PATCH] Fix the missing initializations in __debugfs_file_get()
>
> both method table pointers in debugfs_fsdata need to be initialized,
> obviously, and calculating the bitmap of present methods would also
> go better if we start with initialized state.
>
> Fixes: 41a0ecc0997c "debugfs: get rid of dynamically allocation proxy_ops"
> Fucked-up-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Thanks, now queued up.
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: Regression on linux-next (next-20250120)
2025-01-30 7:25 ` Greg Kroah-Hartman
@ 2025-01-30 8:08 ` Borah, Chaitanya Kumar
2025-01-30 8:36 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Borah, Chaitanya Kumar @ 2025-01-30 8:08 UTC (permalink / raw)
To: Greg Kroah-Hartman, Al Viro
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Kurmi, Suresh Kumar, Saarinen, Jani,
linux-fsdevel@vger.kernel.org, Alexander Gordeev
> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Thursday, January 30, 2025 12:55 PM
> To: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Kurmi, Suresh
> Kumar <suresh.kumar.kurmi@intel.com>; Saarinen, Jani
> <jani.saarinen@intel.com>; linux-fsdevel@vger.kernel.org; Alexander Gordeev
> <agordeev@linux.ibm.com>
> Subject: Re: Regression on linux-next (next-20250120)
>
> On Wed, Jan 29, 2025 at 07:19:37PM +0000, Al Viro wrote:
> > On Wed, Jan 29, 2025 at 08:13:02AM +0100, Greg Kroah-Hartman wrote:
> >
> > > > Both are needed, actually. Slightly longer term I would rather
> > > > split full_proxy_{read,write,lseek}() into short and full variant,
> > > > getting rid of the "check which pointer is non-NULL" and killed
> > > > the two remaining users of debugfs_real_fops() outside of
> > > > fs/debugfs/file.c; then we could union these ->..._fops pointers,
> > > > but until then they need to be initialized.
> > > >
> > > > And yes, ->methods obviously needs to be initialized.
> > > >
> > > > Al, bloody embarrassed ;-/
> > >
> > > No worries, want to send a patch to fix both of these up so we can
> > > fix up Linus's tree now?
> >
> > [PATCH] Fix the missing initializations in __debugfs_file_get()
> >
> > both method table pointers in debugfs_fsdata need to be initialized,
> > obviously, and calculating the bitmap of present methods would also go
> > better if we start with initialized state.
> >
> > Fixes: 41a0ecc0997c "debugfs: get rid of dynamically allocation proxy_ops"
> > Fucked-up-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
>
> Thanks, now queued up.
We can confirm that this change works.
Regards
Chaitanya
>
> greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Regression on linux-next (next-20250120)
2025-01-30 8:08 ` Borah, Chaitanya Kumar
@ 2025-01-30 8:36 ` Greg Kroah-Hartman
0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2025-01-30 8:36 UTC (permalink / raw)
To: Borah, Chaitanya Kumar
Cc: Al Viro, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar,
Saarinen, Jani, linux-fsdevel@vger.kernel.org, Alexander Gordeev
On Thu, Jan 30, 2025 at 08:08:30AM +0000, Borah, Chaitanya Kumar wrote:
>
>
> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Sent: Thursday, January 30, 2025 12:55 PM
> > To: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> > gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Kurmi, Suresh
> > Kumar <suresh.kumar.kurmi@intel.com>; Saarinen, Jani
> > <jani.saarinen@intel.com>; linux-fsdevel@vger.kernel.org; Alexander Gordeev
> > <agordeev@linux.ibm.com>
> > Subject: Re: Regression on linux-next (next-20250120)
> >
> > On Wed, Jan 29, 2025 at 07:19:37PM +0000, Al Viro wrote:
> > > On Wed, Jan 29, 2025 at 08:13:02AM +0100, Greg Kroah-Hartman wrote:
> > >
> > > > > Both are needed, actually. Slightly longer term I would rather
> > > > > split full_proxy_{read,write,lseek}() into short and full variant,
> > > > > getting rid of the "check which pointer is non-NULL" and killed
> > > > > the two remaining users of debugfs_real_fops() outside of
> > > > > fs/debugfs/file.c; then we could union these ->..._fops pointers,
> > > > > but until then they need to be initialized.
> > > > >
> > > > > And yes, ->methods obviously needs to be initialized.
> > > > >
> > > > > Al, bloody embarrassed ;-/
> > > >
> > > > No worries, want to send a patch to fix both of these up so we can
> > > > fix up Linus's tree now?
> > >
> > > [PATCH] Fix the missing initializations in __debugfs_file_get()
> > >
> > > both method table pointers in debugfs_fsdata need to be initialized,
> > > obviously, and calculating the bitmap of present methods would also go
> > > better if we start with initialized state.
> > >
> > > Fixes: 41a0ecc0997c "debugfs: get rid of dynamically allocation proxy_ops"
> > > Fucked-up-by: Al Viro <viro@zeniv.linux.org.uk>
> > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > > ---
> >
> > Thanks, now queued up.
>
> We can confirm that this change works.
Wonderful, thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-01-30 8:37 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-23 15:41 Regression on linux-next (next-20250120) Borah, Chaitanya Kumar
2025-01-23 18:18 ` Al Viro
2025-01-24 13:55 ` Borah, Chaitanya Kumar
2025-01-26 15:54 ` Alexander Gordeev
2025-01-27 5:04 ` Al Viro
2025-01-28 16:00 ` Borah, Chaitanya Kumar
2025-01-29 4:37 ` Al Viro
2025-01-29 7:13 ` Greg Kroah-Hartman
2025-01-29 19:19 ` Al Viro
2025-01-30 7:25 ` Greg Kroah-Hartman
2025-01-30 8:08 ` Borah, Chaitanya Kumar
2025-01-30 8:36 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox