* 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