Regression on linux-next (next-20250120)

Regression on linux-next (next-20250120)

[Borah, Chaitanya Kumar]

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

Re: Regression on linux-next (next-20250120)

[Al Viro]

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?

RE: Regression on linux-next (next-20250120)

[Borah, Chaitanya Kumar]

> -----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

Re: Regression on linux-next (next-20250120)

[Alexander Gordeev]

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!

Re: Regression on linux-next (next-20250120)

[Al Viro]

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);

RE: Regression on linux-next (next-20250120)

[Borah, Chaitanya Kumar]

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

Re: Regression on linux-next (next-20250120)

[Al Viro]

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 ;-/

Re: Regression on linux-next (next-20250120)

[Greg Kroah-Hartman]

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

Re: Regression on linux-next (next-20250120)

[Al Viro]

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);

Re: Regression on linux-next (next-20250120)

[Greg Kroah-Hartman]

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

RE: Regression on linux-next (next-20250120)

[Borah, Chaitanya Kumar]

> -----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

Re: Regression on linux-next (next-20250120)

[Greg Kroah-Hartman]

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!

✗ Fi.CI.BUILD: failure for Regression on linux-next (next-20250120)

[Patchwork]

== Series Details ==

Series: Regression on linux-next (next-20250120)
URL   : https://patchwork.freedesktop.org/series/143978/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/143978/revisions/1/mbox/ not applied
Applying: Regression on linux-next (next-20250120)
Using index info to reconstruct a base tree...
M	fs/debugfs/file.c
Falling back to patching base and 3-way merge...
Auto-merging fs/debugfs/file.c
CONFLICT (content): Merge conflict in fs/debugfs/file.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Regression on linux-next (next-20250120)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced

✗ Fi.CI.BUILD: failure for Regression on linux-next (next-20250120) (rev2)

[Patchwork]

== Series Details ==

Series: Regression on linux-next (next-20250120) (rev2)
URL   : https://patchwork.freedesktop.org/series/143978/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/143978/revisions/2/mbox/ not applied
Applying: Regression on linux-next (next-20250120)
Using index info to reconstruct a base tree...
M	fs/debugfs/file.c
Falling back to patching base and 3-way merge...
Auto-merging fs/debugfs/file.c
CONFLICT (content): Merge conflict in fs/debugfs/file.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Regression on linux-next (next-20250120)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced

Re: ✗ Fi.CI.BUILD: failure for Regression on linux-next (next-20250120) (rev2)

[Al Viro]

On Wed, Jan 29, 2025 at 08:53:37PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: Regression on linux-next (next-20250120) (rev2)
> URL   : https://patchwork.freedesktop.org/series/143978/
> State : failure
> 
> == Summary ==
> 
> Error: patch https://patchwork.freedesktop.org/api/1.0/series/143978/revisions/2/mbox/ not applied
> Applying: Regression on linux-next (next-20250120)
> Using index info to reconstruct a base tree...
> M	fs/debugfs/file.c
> Falling back to patching base and 3-way merge...
> Auto-merging fs/debugfs/file.c
> CONFLICT (content): Merge conflict in fs/debugfs/file.c
> error: Failed to merge in the changes.
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> Patch failed at 0001 Regression on linux-next (next-20250120)
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> Build failed, no error log produced

Not to put too fine a point on that, but... which tree does it fail
to apply to, seeing that both mainline and -next are identical
in fs/debugfs/* and the patch in question applies cleanly to either?

Re: ✗ Fi.CI.BUILD: failure for Regression on linux-next (next-20250120) (rev2)

[Jani Nikula]

On Wed, 29 Jan 2025, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Wed, Jan 29, 2025 at 08:53:37PM -0000, Patchwork wrote:
>> == Series Details ==
>> 
>> Series: Regression on linux-next (next-20250120) (rev2)
>> URL   : https://patchwork.freedesktop.org/series/143978/
>> State : failure
>> 
>> == Summary ==
>> 
>> Error: patch https://patchwork.freedesktop.org/api/1.0/series/143978/revisions/2/mbox/ not applied
>> Applying: Regression on linux-next (next-20250120)
>> Using index info to reconstruct a base tree...
>> M	fs/debugfs/file.c
>> Falling back to patching base and 3-way merge...
>> Auto-merging fs/debugfs/file.c
>> CONFLICT (content): Merge conflict in fs/debugfs/file.c
>> error: Failed to merge in the changes.
>> hint: Use 'git am --show-current-patch=diff' to see the failed patch
>> Patch failed at 0001 Regression on linux-next (next-20250120)
>> When you have resolved this problem, run "git am --continue".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>> Build failed, no error log produced
>
> Not to put too fine a point on that, but... which tree does it fail
> to apply to, seeing that both mainline and -next are identical
> in fs/debugfs/* and the patch in question applies cleanly to either?

drm-tip branch of https://gitlab.freedesktop.org/drm/tip

It's a kind of "linux next for graphics" that we run pre-merge CI on. It
doesn't have other linux-next stuff nor mainline until we backmerge
-rc1. So it doesn't have the stuff you're fixing here either.

Chaitanya & co additionally run tests on linux-next to hopefully catch
regressions before they have a chance to wreak havoc in drm-tip.

Come to think of it, would not be a bad idea to add the baseline info to
these mails. Cc: the CI infra list.


BR,
Jani.


-- 
Jani Nikula, Intel