| Message ID | Z1oKlyKZkSw0Bhq0@duo.ucw.cz (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | 4.4-rt nilfs problem | expand |
> 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
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;
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);
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