linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: hujianyang <hujianyang@huawei.com>
Cc: linux-mtd <linux-mtd@lists.infradead.org>,
	Daniel Golle <daniel@makrotopia.org>
Subject: Re: [PATCH] ubifs: add missing ui pointer in debugging code
Date: Tue, 27 May 2014 15:28:21 +0300	[thread overview]
Message-ID: <1401193701.1304.135.camel@sauron.fi.intel.com> (raw)
In-Reply-To: <53731C00.1020203@huawei.com>

On Wed, 2014-05-14 at 15:32 +0800, hujianyang wrote:
> On 2014/5/14 4:27, Daniel Golle wrote:
> > If UBIFS_DEBUG is defined an additional assertion of the ui_lock
> > spinlock in do_writepage cannot compile because the ui pointer has not
> > been previously declared.
> > 
> > Fix this by declaring and initializing the ui pointer in case
> > UBIFS_DEBUG is defined.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  fs/ubifs/file.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> > index 4f34dba..e13ec5e 100644
> > --- a/fs/ubifs/file.c
> > +++ b/fs/ubifs/file.c
> > @@ -903,6 +903,7 @@ static int do_writepage(struct page *page, int len)
> >  	struct ubifs_info *c = inode->i_sb->s_fs_info;
> >  
> >  #ifdef UBIFS_DEBUG
> > +	struct ubifs_inode *ui = ubifs_inode(inode);
> >  	spin_lock(&ui->ui_lock);
> >  	ubifs_assert(page->index <= ui->synced_i_size << PAGE_CACHE_SIZE);
> 
> I think here should be:
>   	ubifs_assert(page->index <= ui->synced_i_size >> PAGE_CACHE_SHIFT);

I think you are right, thanks. I've just applied this patch:

Author: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date:   Tue May 27 15:24:39 2014 +0300

    UBIFS: fix debugging check
    
    The debugging check which verifies that we never write outside of the file
    length was incorrect, since it was multiplying file length by the page size,
    instead of dividing. Fix this.
    
    Spotted-by: hujianyang <hujianyang@huawei.com>
    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 727506b..2767ffb 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -905,7 +905,7 @@ static int do_writepage(struct page *page, int len)
 #ifdef UBIFS_DEBUG
        struct ubifs_inode *ui = ubifs_inode(inode);
        spin_lock(&ui->ui_lock);
-       ubifs_assert(page->index <= ui->synced_i_size << PAGE_CACHE_SIZE);
+       ubifs_assert(page->index <= ui->synced_i_size >> PAGE_CACHE_SIZE);
        spin_unlock(&ui->ui_lock);
 #endif

-- 
Best Regards,
Artem Bityutskiy

  parent reply	other threads:[~2014-05-27 12:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 20:27 [PATCH] ubifs: add missing ui pointer in debugging code Daniel Golle
2014-05-14  7:32 ` hujianyang
2014-05-15 15:54   ` Daniel
2014-05-27 12:28   ` Artem Bityutskiy [this message]
2014-05-28  2:27     ` hujianyang
2014-05-28  8:04       ` Artem Bityutskiy
2014-05-27 12:27 ` 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=1401193701.1304.135.camel@sauron.fi.intel.com \
    --to=dedekind1@gmail.com \
    --cc=daniel@makrotopia.org \
    --cc=hujianyang@huawei.com \
    --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).