| Message ID | 20190725205627.31313-1-richard@nod.at |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ubifs: Correctly initialize c->min_log_bytes | expand |
----- Ursprüngliche Mail ----- > Von: "richard" <richard@nod.at> > An: "linux-mtd" <linux-mtd@lists.infradead.org> > CC: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>, "richard" <richard@nod.at> > Gesendet: Donnerstag, 25. Juli 2019 22:56:27 > Betreff: [PATCH] ubifs: Correctly initialize c->min_log_bytes > Currently on a freshly mounted UBIFS, c->min_log_bytes is 0. > This can lead to a log overrun and make commits fail. > > Recent kernels will report the following assert: > UBIFS assert failed: c->lhead_lnum != c->ltail_lnum, in fs/ubifs/log.c:412 > > c->min_log_bytes can have two states, 0 and c->leb_size. > It controls how much bytes of the log area are reserved for non-bud > nodes such as commit nodes. > > After a commit it has to be set to c->leb_size such that we have always > enought space for a commit. While a commit runs it can be 0 to make the > remaining bytes of the log available to writers. > > Having it set to 0 right after mount is wrong since no space for commits > is reserved. > > Reported-and-tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Signed-off-by: Richard Weinberger <richard@nod.at> Fixes: 1e51764a3c2ac ("UBIFS: add new flash file system")
Hello Richard, On Thu, Jul 25, 2019 at 10:56:27PM +0200, Richard Weinberger wrote: > Currently on a freshly mounted UBIFS, c->min_log_bytes is 0. > This can lead to a log overrun and make commits fail. > > Recent kernels will report the following assert: > UBIFS assert failed: c->lhead_lnum != c->ltail_lnum, in fs/ubifs/log.c:412 > > c->min_log_bytes can have two states, 0 and c->leb_size. > It controls how much bytes of the log area are reserved for non-bud > nodes such as commit nodes. > > After a commit it has to be set to c->leb_size such that we have always > enought space for a commit. While a commit runs it can be 0 to make the s/enought/enough/ > remaining bytes of the log available to writers. > > Having it set to 0 right after mount is wrong since no space for commits > is reserved. > > Reported-and-tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Signed-off-by: Richard Weinberger <richard@nod.at> Thanks for your support Uwe
Hello Richard, On Thu, Jul 25, 2019 at 10:56:27PM +0200, Richard Weinberger wrote: > Currently on a freshly mounted UBIFS, c->min_log_bytes is 0. > This can lead to a log overrun and make commits fail. > > Recent kernels will report the following assert: > UBIFS assert failed: c->lhead_lnum != c->ltail_lnum, in fs/ubifs/log.c:412 > > c->min_log_bytes can have two states, 0 and c->leb_size. > It controls how much bytes of the log area are reserved for non-bud > nodes such as commit nodes. > > After a commit it has to be set to c->leb_size such that we have always > enought space for a commit. While a commit runs it can be 0 to make the > remaining bytes of the log available to writers. > > Having it set to 0 right after mount is wrong since no space for commits > is reserved. > > Reported-and-tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Signed-off-by: Richard Weinberger <richard@nod.at> I assume you care for applying this to mainline? I wonder it isn't in next yet?! Best regards Uwe
----- Ursprüngliche Mail ----- > Von: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> > An: "richard" <richard@nod.at> > CC: "linux-mtd" <linux-mtd@lists.infradead.org> > Gesendet: Mittwoch, 31. Juli 2019 09:42:44 > Betreff: Re: [PATCH] ubifs: Correctly initialize c->min_log_bytes > Hello Richard, > > On Thu, Jul 25, 2019 at 10:56:27PM +0200, Richard Weinberger wrote: >> Currently on a freshly mounted UBIFS, c->min_log_bytes is 0. >> This can lead to a log overrun and make commits fail. >> >> Recent kernels will report the following assert: >> UBIFS assert failed: c->lhead_lnum != c->ltail_lnum, in fs/ubifs/log.c:412 >> >> c->min_log_bytes can have two states, 0 and c->leb_size. >> It controls how much bytes of the log area are reserved for non-bud >> nodes such as commit nodes. >> >> After a commit it has to be set to c->leb_size such that we have always >> enought space for a commit. While a commit runs it can be 0 to make the >> remaining bytes of the log available to writers. >> >> Having it set to 0 right after mount is wrong since no space for commits >> is reserved. >> >> Reported-and-tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >> Signed-off-by: Richard Weinberger <richard@nod.at> > > I assume you care for applying this to mainline? I wonder it isn't in > next yet?! Don't stress me. The issue is present in UBIFS since day 0, there is absolutely no reason to hurry. It will get merged when I'm back from traveling and my time permits it. Thanks, //richard
Hello Richard, On Wed, Jul 31, 2019 at 10:07:06AM +0200, Richard Weinberger wrote: > > On Thu, Jul 25, 2019 at 10:56:27PM +0200, Richard Weinberger wrote: > >> Currently on a freshly mounted UBIFS, c->min_log_bytes is 0. > >> This can lead to a log overrun and make commits fail. > >> > >> Recent kernels will report the following assert: > >> UBIFS assert failed: c->lhead_lnum != c->ltail_lnum, in fs/ubifs/log.c:412 > >> > >> c->min_log_bytes can have two states, 0 and c->leb_size. > >> It controls how much bytes of the log area are reserved for non-bud > >> nodes such as commit nodes. > >> > >> After a commit it has to be set to c->leb_size such that we have always > >> enought space for a commit. While a commit runs it can be 0 to make the > >> remaining bytes of the log available to writers. > >> > >> Having it set to 0 right after mount is wrong since no space for commits > >> is reserved. > >> > >> Reported-and-tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > >> Signed-off-by: Richard Weinberger <richard@nod.at> > > > > I assume you care for applying this to mainline? I wonder it isn't in > > next yet?! > > Don't stress me. The issue is present in UBIFS since day 0, there is absolutely > no reason to hurry. > It will get merged when I'm back from traveling and my time permits it. My intention was not to stess you, just to ensure this is eventually taken care of and to check if there is a blocker that I can help to remove. Thanks Uwe
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 2c0803b0ac3a..8c1d571334bc 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -609,6 +609,10 @@ static int init_constants_early(struct ubifs_info *c) c->max_bu_buf_len = UBIFS_MAX_BULK_READ * UBIFS_MAX_DATA_NODE_SZ; if (c->max_bu_buf_len > c->leb_size) c->max_bu_buf_len = c->leb_size; + + /* Log is ready, preserve one LEB for commits. */ + c->min_log_bytes = c->leb_size; + return 0; }
Currently on a freshly mounted UBIFS, c->min_log_bytes is 0. This can lead to a log overrun and make commits fail. Recent kernels will report the following assert: UBIFS assert failed: c->lhead_lnum != c->ltail_lnum, in fs/ubifs/log.c:412 c->min_log_bytes can have two states, 0 and c->leb_size. It controls how much bytes of the log area are reserved for non-bud nodes such as commit nodes. After a commit it has to be set to c->leb_size such that we have always enought space for a commit. While a commit runs it can be 0 to make the remaining bytes of the log available to writers. Having it set to 0 right after mount is wrong since no space for commits is reserved. Reported-and-tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/super.c | 4 ++++ 1 file changed, 4 insertions(+)