4.4-rt nilfs problem

4.4-rt nilfs problem

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

Hi!

This 4.4-rt release is proving more "fun" then average. Last problem I
hit is this one:

https://gitlab.com/cip-project/cip-kernel/linux-cip/-/jobs/8612057735

fs/nilfs2/dir.c: In function 'nilfs_find_entry':
2705
fs/nilfs2/dir.c:350:31: error: initialization of 'char *' from incompatible pointer type 'struct page *' [-Werror=incompatible-pointer-types]
2706
  350 |                 char *kaddr = nilfs_get_page(dir, n);
2707
      |                               ^~~~~~~~~~~~~~
2708

And I believe it is caused by 3ab089df2d67491785a5bdaab01cfab8eb35dab5
:

    nilfs2: propagate directory read errors from nilfs_find_entry()

@@ -345,25 +347,26 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
                start = 0;
        n = start;
        do {
-               char *kaddr;
-               page = nilfs_get_page(dir, n);
-               if (!IS_ERR(page)) {
-                       kaddr = page_address(page);
...
+               char *kaddr = nilfs_get_page(dir, n);

Compiler does not like page * assigned into char *, and I don't blame
him.

I could probably hack something up -- like below -- but it raises
questions such as

a) "should kaddr by checked for IS_ERR like that"?

and

b) "am I comfortable changing code we are not testing"?

Best regards,
								Pavel

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 61ca998ac8013..ea0df90626a0d 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -347,8 +347,13 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
 		start = 0;
 	n = start;
 	do {
-		char *kaddr = nilfs_get_page(dir, n);
+		char *kaddr;
+		
+		page = nilfs_get_page(dir, n);
+		if (IS_ERR(page))
+			return ERR_CAST(page);
 
+		kaddr = page_address(page);
 		if (IS_ERR(kaddr))
 			return ERR_CAST(kaddr);
 



-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: 4.4-rt nilfs problem

[Ulrich Hecht]

> On 12/11/2024 10:56 PM CET Pavel Machek <pavel@denx.de> wrote:
> This 4.4-rt release is proving more "fun" then average. Last problem I
> hit is this one:
> 
> https://gitlab.com/cip-project/cip-kernel/linux-cip/-/jobs/8612057735
> 
> fs/nilfs2/dir.c: In function 'nilfs_find_entry':
> 2705
> fs/nilfs2/dir.c:350:31: error: initialization of 'char *' from incompatible pointer type 'struct page *' [-Werror=incompatible-pointer-types]
> 2706
>   350 |                 char *kaddr = nilfs_get_page(dir, n);
> 2707
>       |                               ^~~~~~~~~~~~~~
> 2708
> 
> And I believe it is caused by 3ab089df2d67491785a5bdaab01cfab8eb35dab5
> :
> 
>     nilfs2: propagate directory read errors from nilfs_find_entry()
> 
> @@ -345,25 +347,26 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
>                 start = 0;
>         n = start;
>         do {
> -               char *kaddr;
> -               page = nilfs_get_page(dir, n);
> -               if (!IS_ERR(page)) {
> -                       kaddr = page_address(page);
> ...
> +               char *kaddr = nilfs_get_page(dir, n);
> 
> Compiler does not like page * assigned into char *, and I don't blame
> him.

It seems this is missing a dependency, namely ea2ac9238d4919bdc6963a2b487a65ccdaa11c78 ("nilfs2: return the mapped address from nilfs_get_page()"), which in turn has more dependencies...

> I could probably hack something up -- like below -- but it raises
> questions such as
> 
> a) "should kaddr by checked for IS_ERR like that"?

AFAICT, no. page_address() returns NULL on error.

> b) "am I comfortable changing code we are not testing"?

Given that that is what this backport does and how well that worked, I guess the answer is no...

When maintaining file systems for 4.4, nilfs2 is the biggest effort by a mile. From what I can tell it's used in only one config (moxa_mxc_defconfig). Is there any way to find out if it is actually used or just happens to be enabled?

CU
Uli

Re: [cip-dev] 4.4-rt nilfs problem

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

Hi!

> > Compiler does not like page * assigned into char *, and I don't blame
> > him.
> 
> It seems this is missing a dependency, namely ea2ac9238d4919bdc6963a2b487a65ccdaa11c78 ("nilfs2: return the mapped address from nilfs_get_page()"), which in turn has more dependencies...
> 
> > I could probably hack something up -- like below -- but it raises
> > questions such as
> > 
> > a) "should kaddr by checked for IS_ERR like that"?
> 
> AFAICT, no. page_address() returns NULL on error.

Ok, so I did change below to patch it up. I'll release it as
4.4-cip-rt if I get testing to cooperate. 

> > b) "am I comfortable changing code we are not testing"?
> 
> Given that that is what this backport does and how well that worked, I guess the answer is no...
> 
> When maintaining file systems for 4.4, nilfs2 is the biggest effort by a mile. From what I can tell it's used in only one config (moxa_mxc_defconfig). Is there any way to find out if it is actually used or just happens to be enabled?
>

Ok, I believe backporting more is not a solution here. I don't believe
we do any runtime nilfs2 testing. I guess we should talk about it on
irc (am I wrong and somebody is using/testing this?), but I believe we
should simply avoid this kind of intrusive changes. It is "only" a
robustness in case of error, so not something I'll lose sleep over.

Best regards,
								Pavel


commit 193ce60475afc15ceeb572c4f750c1d793e598ae
Author: Pavel Machek <pavel@ucw.cz>
Date:   Wed Dec 11 22:58:24 2024 +0100

    nilfs2: fix backport of "propagate directory read errors from nilfs_find_entry()"
    
    I believe 3ab089df2d674 was backported wrong. Types are confused and
    that causes compile error. This fixes compile problems.
    
    Signed-off-by: Pavel Machek <pavel@denx.de>

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 61ca998ac8013..9b9e6235819e2 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -347,10 +347,15 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
 		start = 0;
 	n = start;
 	do {
-		char *kaddr = nilfs_get_page(dir, n);
+		char *kaddr;
 
-		if (IS_ERR(kaddr))
-			return ERR_CAST(kaddr);
+		page = nilfs_get_page(dir, n);
+		if (IS_ERR(page))
+			return ERR_CAST(page);
+
+		kaddr = page_address(page);
+		if (!kaddr)
+			return ERR_PTR(-ENOMEM);
 
 		de = (struct nilfs_dir_entry *)kaddr;
 		kaddr += nilfs_last_byte(dir, n) - reclen;



-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]