linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Dolev Raviv <draviv@codeaurora.org>
Cc: linux-mtd@lists.infradead.org
Subject: Re: ubifs: assertion fails
Date: Mon, 31 Mar 2014 13:14:39 +0300	[thread overview]
Message-ID: <1396260879.9016.70.camel@sauron.fi.intel.com> (raw)
In-Reply-To: <f08bbebef039d5bb28c2fcd3022165b0.squirrel@www.codeaurora.org>

On Mon, 2014-03-24 at 06:03 +0000, Dolev Raviv wrote:
> Hi all,
> 
> I’m doing my first steps learning ubifs and I’m trying to understand a
> something that does not make much sense to me.
> 
> In fs/ubifs/shrinker.c, at shrink_tnc(), there is an assert condition that
> shows up every once I a while (after stressing).
> ubifs_assert(atomic_long_read(&c->clean_zn_cnt) >= 0);

When this happens, do you then see a storm of similar assertions from
other parts of the code? I am trying to understand if this assertion is
incorrect, or you really get the accounting screwed when shrinking
happens.

In the former case, this would probably be a single assertion, on the
latter you'd probably see many similar warnings from other code. E.g.,
when you unmount.

> In another place in the same file in the function ubifs_shrinker(), I
> found the following comment:
> /*
> * Due to the way UBIFS updates the clean znode counter it may
> * temporarily be negative.
> */

Yeah. The key here is this 'c->next'. If it is NULL, the accounting must
be correct, if it is not NULL, it may be incorrect. And it will be
correct when the on-going commit operation finishes and
'free_obsolete_znodes()' is called.

> Could the assertion condition be wrong?

Could be, but could also show that there is an accounting error
happening when shrinker starts.

And I saw misterious errors when shrinker starts working at some point,
but did not have time to dig this. So there is at least 1 bug in the
shrinker path which I saw.

> Can anyone share information on what are those times that the counter can
> be negative?

When the commit operation starts, it grabs the tnc_mutex, prepares the
list of nodes to commit, and release tnc_mutex. Now the accounting is
incorrect. When the commit finishes, it grabs the mutex again, does some
stuff, and also fixes the accounting. Then drops the mutex.

The idea was to make sure that commit does not block I/O. Meaning that
you can still write files while commit is going on.

-- 
Best Regards,
Artem Bityutskiy

  reply	other threads:[~2014-03-31 10:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-24  6:03 ubifs: assertion fails Dolev Raviv
2014-03-31 10:14 ` Artem Bityutskiy [this message]
2014-04-01  5:52   ` Tanya Brokhman
2014-04-07 11:43     ` Artem Bityutskiy
2014-04-27 12:12       ` Dolev Raviv
2014-05-29  1:55         ` hujianyang
2014-05-29  7:24           ` Dolev Raviv
2014-05-29  7:42           ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1396260879.9016.70.camel@sauron.fi.intel.com \
    --to=dedekind1@gmail.com \
    --cc=draviv@codeaurora.org \
    --cc=linux-mtd@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).