Hi,
On Tue, Dec 03, 2019 at 04:52:32AM +0000, Naga Sureshkumar Relli wrote:
> Hi,
>
>
>
> We have upgraded our Linux kernel to 5.4 from 4.19.
>
> And I tried mounting ubifs using this kernel on NAND partition with below
> command and saw that
>
> There is an issue with memory allocation.
>
> #mount -t ubifs ubi0:data /mnt/
>
> mount: mounting ubi0:data on /mnt/ failed: Cannot allocate memory
>
>
>
> I saw that there is a commit on fs/ubifs/sb.c, where it is allocating all
> the required memories at one shot.
>
> [1]https://lkml.org/lkml/2018/9/7/724
>
> By reverting the above patch, I am able to mount successfully the ubifs.
>
> By reverting this patch, we are allocating, writing and freeing in a
> manner such that, we don’t see memory allocation issues.
Sorry, I can't see how this patch causes failing memory allocations. And
no, this is not expected. Could you sprinkle some printks and track down
where it fails? Is it the obvious place here:
if (!sup || !mst || !idx || !ino || !cs) {
err = -ENOMEM;
goto out;
}
If yes, which allocation fails and how much memory did we try to allocate?
If no, where does it fail? Also, where are you using UBIFS. Is it NAND flash
or NOR flash?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Sascha,
> -----Original Message-----
> From: Sascha Hauer <[email protected]>
> Sent: Tuesday, December 3, 2019 2:12 PM
> To: Naga Sureshkumar Relli <[email protected]>
> Cc: Richard Weinberger <[email protected]>; Miquel Raynal <[email protected]>;
> [email protected]; [email protected]; Michal Simek
> <[email protected]>; [email protected]
> Subject: Re: ubifs mount failure
>
> Hi,
>
> On Tue, Dec 03, 2019 at 04:52:32AM +0000, Naga Sureshkumar Relli wrote:
> > Hi,
> >
> >
> >
> > We have upgraded our Linux kernel to 5.4 from 4.19.
> >
> > And I tried mounting ubifs using this kernel on NAND partition with below
> > command and saw that
> >
> > There is an issue with memory allocation.
> >
> > #mount -t ubifs ubi0:data /mnt/
> >
> > mount: mounting ubi0:data on /mnt/ failed: Cannot allocate memory
> >
> >
> >
> > I saw that there is a commit on fs/ubifs/sb.c, where it is allocating all
> > the required memories at one shot.
> >
> > [1]https://lkml.org/lkml/2018/9/7/724
> >
> > By reverting the above patch, I am able to mount successfully the ubifs.
> >
> > By reverting this patch, we are allocating, writing and freeing in a
> > manner such that, we don’t see memory allocation issues.
>
> Sorry, I can't see how this patch causes failing memory allocations. And no, this is not
> expected. Could you sprinkle some printks and track down where it fails? Is it the obvious
> place here:
Yes, it is failing in this place only.
>
> if (!sup || !mst || !idx || !ino || !cs) {
> err = -ENOMEM;
> goto out;
> }
>
> If yes, which allocation fails and how much memory did we try to allocate?
Failing at indexing node(idx) allocation
https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
we are trying to allocate 4325376 (~4MB)
> If no, where does it fail? Also, where are you using UBIFS. Is it NAND flash or NOR flash?
It is on NAND flash.
Thanks,
Naga Sureshkumar Relli
>
> Sascha
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
----- Ursprüngliche Mail -----
> Von: "Naga Sureshkumar Relli" <[email protected]>
> https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
> we are trying to allocate 4325376 (~4MB)
4MiB? Is ->min_io_size that large?
Thanks,
//richard
Hi Richard,
On Tue, Dec 3, 2019 at 2:40 PM Richard Weinberger <[email protected]> wrote:
>
> ----- Ursprüngliche Mail -----
> > Von: "Naga Sureshkumar Relli" <[email protected]>
> > https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
> > we are trying to allocate 4325376 (~4MB)
>
> 4MiB? Is ->min_io_size that large?
if you see https://elixir.bootlin.com/linux/latest/source/fs/ubifs/sb.c#L164
The size is actually ALIGN(tmp, c->min_io_size).
Here tmp is of 4325376 Bytes and min_io_size is 16384 Bytes
Thanks,
Naga Sureshkumar Relli
>
> Thanks,
> //richard
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, Dec 03, 2019 at 04:06:12PM +0530, naga suresh kumar wrote:
> Hi Richard,
>
> On Tue, Dec 3, 2019 at 2:40 PM Richard Weinberger <[email protected]> wrote:
> >
> > ----- Urspr?ngliche Mail -----
> > > Von: "Naga Sureshkumar Relli" <[email protected]>
> > > https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
> > > we are trying to allocate 4325376 (~4MB)
> >
> > 4MiB? Is ->min_io_size that large?
> if you see https://elixir.bootlin.com/linux/latest/source/fs/ubifs/sb.c#L164
> The size is actually ALIGN(tmp, c->min_io_size).
> Here tmp is of 4325376 Bytes and min_io_size is 16384 Bytes
'tmp' contains bogus values. Try this:
----------------------------8<--------------------------------
From 34f687fce189085f55706b4cddcb288a08f4ee06 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <[email protected]>
Date: Tue, 3 Dec 2019 11:41:20 +0100
Subject: [PATCH] ubifs: Fix wrong memory allocation
In create_default_filesystem() when we allocate the idx node we must use
the idx_node_size we calculated just one line before, not tmp, which
contains completely other data.
Fixes: c4de6d7e4319 ("ubifs: Refactor create_default_filesystem()")
Reported-by: Naga Sureshkumar Relli <[email protected]>
Signed-off-by: Sascha Hauer <[email protected]>
---
fs/ubifs/sb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index a551eb3e9b89..6681c18e52b8 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -161,7 +161,7 @@ static int create_default_filesystem(struct ubifs_info *c)
sup = kzalloc(ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size), GFP_KERNEL);
mst = kzalloc(c->mst_node_alsz, GFP_KERNEL);
idx_node_size = ubifs_idx_node_sz(c, 1);
- idx = kzalloc(ALIGN(tmp, c->min_io_size), GFP_KERNEL);
+ idx = kzalloc(ALIGN(idx_node_size, c->min_io_size), GFP_KERNEL);
ino = kzalloc(ALIGN(UBIFS_INO_NODE_SZ, c->min_io_size), GFP_KERNEL);
cs = kzalloc(ALIGN(UBIFS_CS_NODE_SZ, c->min_io_size), GFP_KERNEL);
--
2.24.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Sascha,
Tested this patch. and it worked.
Thanks for your quick response.
On Tue, Dec 3, 2019 at 4:16 PM Sascha Hauer <[email protected]> wrote:
>
> On Tue, Dec 03, 2019 at 04:06:12PM +0530, naga suresh kumar wrote:
> > Hi Richard,
> >
> > On Tue, Dec 3, 2019 at 2:40 PM Richard Weinberger <[email protected]> wrote:
> > >
> > > ----- Ursprüngliche Mail -----
> > > > Von: "Naga Sureshkumar Relli" <[email protected]>
> > > > https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
> > > > we are trying to allocate 4325376 (~4MB)
> > >
> > > 4MiB? Is ->min_io_size that large?
> > if you see https://elixir.bootlin.com/linux/latest/source/fs/ubifs/sb.c#L164
> > The size is actually ALIGN(tmp, c->min_io_size).
> > Here tmp is of 4325376 Bytes and min_io_size is 16384 Bytes
>
> 'tmp' contains bogus values. Try this:
>
> ----------------------------8<--------------------------------
>
> From 34f687fce189085f55706b4cddcb288a08f4ee06 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <[email protected]>
> Date: Tue, 3 Dec 2019 11:41:20 +0100
> Subject: [PATCH] ubifs: Fix wrong memory allocation
>
> In create_default_filesystem() when we allocate the idx node we must use
> the idx_node_size we calculated just one line before, not tmp, which
> contains completely other data.
>
> Fixes: c4de6d7e4319 ("ubifs: Refactor create_default_filesystem()")
> Reported-by: Naga Sureshkumar Relli <[email protected]>
> Signed-off-by: Sascha Hauer <[email protected]>
> ---
> fs/ubifs/sb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
> index a551eb3e9b89..6681c18e52b8 100644
> --- a/fs/ubifs/sb.c
> +++ b/fs/ubifs/sb.c
> @@ -161,7 +161,7 @@ static int create_default_filesystem(struct ubifs_info *c)
> sup = kzalloc(ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size), GFP_KERNEL);
> mst = kzalloc(c->mst_node_alsz, GFP_KERNEL);
> idx_node_size = ubifs_idx_node_sz(c, 1);
> - idx = kzalloc(ALIGN(tmp, c->min_io_size), GFP_KERNEL);
> + idx = kzalloc(ALIGN(idx_node_size, c->min_io_size), GFP_KERNEL);
> ino = kzalloc(ALIGN(UBIFS_INO_NODE_SZ, c->min_io_size), GFP_KERNEL);
> cs = kzalloc(ALIGN(UBIFS_CS_NODE_SZ, c->min_io_size), GFP_KERNEL);
>
> --
> 2.24.0
>
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Thanks,
Naga Sureshkumar Relli.
On Tue, Dec 3, 2019 at 11:46 AM Sascha Hauer <[email protected]> wrote:
>
> On Tue, Dec 03, 2019 at 04:06:12PM +0530, naga suresh kumar wrote:
> > Hi Richard,
> >
> > On Tue, Dec 3, 2019 at 2:40 PM Richard Weinberger <[email protected]> wrote:
> > >
> > > ----- Ursprüngliche Mail -----
> > > > Von: "Naga Sureshkumar Relli" <[email protected]>
> > > > https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
> > > > we are trying to allocate 4325376 (~4MB)
> > >
> > > 4MiB? Is ->min_io_size that large?
> > if you see https://elixir.bootlin.com/linux/latest/source/fs/ubifs/sb.c#L164
> > The size is actually ALIGN(tmp, c->min_io_size).
> > Here tmp is of 4325376 Bytes and min_io_size is 16384 Bytes
>
> 'tmp' contains bogus values. Try this:
>
> ----------------------------8<--------------------------------
>
> From 34f687fce189085f55706b4cddcb288a08f4ee06 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <[email protected]>
> Date: Tue, 3 Dec 2019 11:41:20 +0100
> Subject: [PATCH] ubifs: Fix wrong memory allocation
>
> In create_default_filesystem() when we allocate the idx node we must use
> the idx_node_size we calculated just one line before, not tmp, which
> contains completely other data.
>
> Fixes: c4de6d7e4319 ("ubifs: Refactor create_default_filesystem()")
> Reported-by: Naga Sureshkumar Relli <[email protected]>
> Signed-off-by: Sascha Hauer <[email protected]>
> ---
> fs/ubifs/sb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
> index a551eb3e9b89..6681c18e52b8 100644
> --- a/fs/ubifs/sb.c
> +++ b/fs/ubifs/sb.c
> @@ -161,7 +161,7 @@ static int create_default_filesystem(struct ubifs_info *c)
> sup = kzalloc(ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size), GFP_KERNEL);
> mst = kzalloc(c->mst_node_alsz, GFP_KERNEL);
> idx_node_size = ubifs_idx_node_sz(c, 1);
> - idx = kzalloc(ALIGN(tmp, c->min_io_size), GFP_KERNEL);
> + idx = kzalloc(ALIGN(idx_node_size, c->min_io_size), GFP_KERNEL);
> ino = kzalloc(ALIGN(UBIFS_INO_NODE_SZ, c->min_io_size), GFP_KERNEL);
> cs = kzalloc(ALIGN(UBIFS_CS_NODE_SZ, c->min_io_size), GFP_KERNEL);
Oh, looks good! Thanks for fixing, Sascha!
Thanks,
//richard
On Tue, Dec 03, 2019 at 08:08:48PM +0100, Richard Weinberger wrote:
> On Tue, Dec 3, 2019 at 11:46 AM Sascha Hauer <[email protected]> wrote:
> >
> > On Tue, Dec 03, 2019 at 04:06:12PM +0530, naga suresh kumar wrote:
> > > Hi Richard,
> > >
> > > On Tue, Dec 3, 2019 at 2:40 PM Richard Weinberger <[email protected]> wrote:
> > > >
> > > > ----- Urspr?ngliche Mail -----
> > > > > Von: "Naga Sureshkumar Relli" <[email protected]>
> > > > > https://elixir.bootlin.com/linux/v5.4/source/fs/ubifs/sb.c#L164
> > > > > we are trying to allocate 4325376 (~4MB)
> > > >
> > > > 4MiB? Is ->min_io_size that large?
> > > if you see https://elixir.bootlin.com/linux/latest/source/fs/ubifs/sb.c#L164
> > > The size is actually ALIGN(tmp, c->min_io_size).
> > > Here tmp is of 4325376 Bytes and min_io_size is 16384 Bytes
> >
> > 'tmp' contains bogus values. Try this:
> >
> > ----------------------------8<--------------------------------
> >
> > From 34f687fce189085f55706b4cddcb288a08f4ee06 Mon Sep 17 00:00:00 2001
> > From: Sascha Hauer <[email protected]>
> > Date: Tue, 3 Dec 2019 11:41:20 +0100
> > Subject: [PATCH] ubifs: Fix wrong memory allocation
> >
> > In create_default_filesystem() when we allocate the idx node we must use
> > the idx_node_size we calculated just one line before, not tmp, which
> > contains completely other data.
> >
> > Fixes: c4de6d7e4319 ("ubifs: Refactor create_default_filesystem()")
> > Reported-by: Naga Sureshkumar Relli <[email protected]>
> > Signed-off-by: Sascha Hauer <[email protected]>
> > ---
> > fs/ubifs/sb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
> > index a551eb3e9b89..6681c18e52b8 100644
> > --- a/fs/ubifs/sb.c
> > +++ b/fs/ubifs/sb.c
> > @@ -161,7 +161,7 @@ static int create_default_filesystem(struct ubifs_info *c)
> > sup = kzalloc(ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size), GFP_KERNEL);
> > mst = kzalloc(c->mst_node_alsz, GFP_KERNEL);
> > idx_node_size = ubifs_idx_node_sz(c, 1);
> > - idx = kzalloc(ALIGN(tmp, c->min_io_size), GFP_KERNEL);
> > + idx = kzalloc(ALIGN(idx_node_size, c->min_io_size), GFP_KERNEL);
> > ino = kzalloc(ALIGN(UBIFS_INO_NODE_SZ, c->min_io_size), GFP_KERNEL);
> > cs = kzalloc(ALIGN(UBIFS_CS_NODE_SZ, c->min_io_size), GFP_KERNEL);
>
> Oh, looks good! Thanks for fixing, Sascha!
Will you apply this one? Otherwise I resend with the proper tags added.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
----- Ursprüngliche Mail -----
> Von: "Sascha Hauer" <[email protected]>
>> Oh, looks good! Thanks for fixing, Sascha!
>
> Will you apply this one? Otherwise I resend with the proper tags added.
Just checked in patchwork. It is unable to detect the patch, please resend. :-)
Thanks,
//richard
On Tue, Dec 03, 2019 at 04:23:34PM +0530, naga suresh kumar wrote:
> Hi Sascha,
>
> Tested this patch. and it worked.
> Thanks for your quick response.
Ok, I take this as a:
Tested-by: Naga Sureshkumar Relli <[email protected]>
Thanks
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |