diff mbox

[2/2] UBIFS: fix-up free space on mount if flag is set

Message ID 1304722703-7904-3-git-send-email-mlcreech@gmail.com
State New, archived
Headers show

Commit Message

Matthew L. Creech May 6, 2011, 10:58 p.m. UTC
If a UBIFS filesystem is being mounted read-write, or is being remounted
from read-only to read-write, check for the "space_fixup" flag and fix
all LEBs containing empty space if necessary.

Signed-off-by: Matthew L. Creech <mlcreech@gmail.com>
---
 fs/ubifs/super.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

Comments

Artem Bityutskiy May 12, 2011, 10:57 a.m. UTC | #1
On Fri, 2011-05-06 at 18:58 -0400, Matthew L. Creech wrote:
> If a UBIFS filesystem is being mounted read-write, or is being remounted
> from read-only to read-write, check for the "space_fixup" flag and fix
> all LEBs containing empty space if necessary.
> 
> Signed-off-by: Matthew L. Creech <mlcreech@gmail.com>

Pushed to l2-mtd-2.6.git, thanks!

Also, it would be nice to have a piece of documentation for this feature
in the mtd web site - could you cook a patch for that too please? And
please, add links to this doc from other relevant places - you posted
links AFAIR.
Artem Bityutskiy May 12, 2011, 11:09 a.m. UTC | #2
On Fri, 2011-05-06 at 18:58 -0400, Matthew L. Creech wrote:
> +	if (c->space_fixup) {
> +		err = ubifs_fixup_free_space(c);
> +		if (err)
> +			goto out;
> +	}
> +
>  	dbg_gen("re-mounted read-write");
>  	c->remounting_rw = 0;
>  	err = dbg_check_space_info(c);

Note, I've moved the 'ubifs_fixup_free_space()' a bit down to make sure
we first print "re-mounted" and "deferred recovery completed" messages,
and then start fixing up. I think this is a bit neater.
Artem Bityutskiy May 13, 2011, 7:58 a.m. UTC | #3
On Thu, 2011-05-12 at 14:09 +0300, Artem Bityutskiy wrote:
> On Fri, 2011-05-06 at 18:58 -0400, Matthew L. Creech wrote:
> > +	if (c->space_fixup) {
> > +		err = ubifs_fixup_free_space(c);
> > +		if (err)
> > +			goto out;
> > +	}
> > +
> >  	dbg_gen("re-mounted read-write");
> >  	c->remounting_rw = 0;
> >  	err = dbg_check_space_info(c);
> 
> Note, I've moved the 'ubifs_fixup_free_space()' a bit down to make sure
> we first print "re-mounted" and "deferred recovery completed" messages,
> and then start fixing up. I think this is a bit neater.

I do not know if that was me or you, but I've found out that the a lot
of code uses spaces for indentation instead of tabs, and checkpatch.pl
complains:

WARNING: please, no spaces at the start of a line
#119: FILE: fs/ubifs/sb.c:746:
+                       err = PTR_ERR(lprops);$

ERROR: code indent should use tabs where possible
#120: FILE: fs/ubifs/sb.c:747:
+                       goto out;$

.... a lot of this ....

But I've just fixed this.
Atlant Schmidt May 13, 2011, 10:59 a.m. UTC | #4
Artem:

  I don't know what the coding standards are like on
  your project, but in very much of the C/C++ developer
  world, <tab>s are frowned upon in code as being fragile,
  primarily because there are varying standards for just
  how wide a <tab> should be. By default, they're 8 spaces,
  but I've worked at several shops where they are only
  4 spaces and one shop where they are deemed to be 3!

  Because of this, many coding standards (such as the
  WebKit coding standards and my current employer's
  standard) outright ban <tab> characters in code and
  I'd recommend you do so as well, no matter what the
  current scripts enforce. (It's certainly easy enough
  to expand all the tabs in an existing code base so
  as to bring it into compliance with a "no tabs" rule.)

                            Atlant

-----Original Message-----
From: linux-mtd-bounces@lists.infradead.org [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of Artem Bityutskiy
Sent: Friday, May 13, 2011 03:59
To: Matthew L. Creech
Cc: linux-mtd@lists.infradead.org
Subject: Re: [PATCH 2/2] UBIFS: fix-up free space on mount if flag is set

On Thu, 2011-05-12 at 14:09 +0300, Artem Bityutskiy wrote:
> On Fri, 2011-05-06 at 18:58 -0400, Matthew L. Creech wrote:
> > +   if (c->space_fixup) {
> > +           err = ubifs_fixup_free_space(c);
> > +           if (err)
> > +                   goto out;
> > +   }
> > +
> >     dbg_gen("re-mounted read-write");
> >     c->remounting_rw = 0;
> >     err = dbg_check_space_info(c);
>
> Note, I've moved the 'ubifs_fixup_free_space()' a bit down to make sure
> we first print "re-mounted" and "deferred recovery completed" messages,
> and then start fixing up. I think this is a bit neater.

I do not know if that was me or you, but I've found out that the a lot
of code uses spaces for indentation instead of tabs, and checkpatch.pl
complains:

WARNING: please, no spaces at the start of a line
#119: FILE: fs/ubifs/sb.c:746:
+                       err = PTR_ERR(lprops);$

ERROR: code indent should use tabs where possible
#120: FILE: fs/ubifs/sb.c:747:
+                       goto out;$

.... a lot of this ....

But I've just fixed this.

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
Michael Cashwell May 13, 2011, 12:02 p.m. UTC | #5
Any many shops/projects feel strongly the other way.

I don't buy the "tabs are fragile" argument. It makes no sense. Even if a project uses spaces but some people indent to 8, some to 4 and some to 3 you still end up with a hard-to-read mess. Mandating the use of spaces is solving the wrong problem.

The problem is that there must be agreement on what the indentation value is. Once you have that it then the code is readable and clean regardless of whether you use spaces or tabs to do it.

-Mike

On May 13, 2011, at 6:59 AM, Atlant Schmidt wrote:

> Artem:
> 
>  I don't know what the coding standards are like on
>  your project, but in very much of the C/C++ developer
>  world, <tab>s are frowned upon in code as being fragile,
>  primarily because there are varying standards for just
>  how wide a <tab> should be. By default, they're 8 spaces,
>  but I've worked at several shops where they are only
>  4 spaces and one shop where they are deemed to be 3!
> 
>  Because of this, many coding standards (such as the
>  WebKit coding standards and my current employer's
>  standard) outright ban <tab> characters in code and
>  I'd recommend you do so as well, no matter what the
>  current scripts enforce. (It's certainly easy enough
>  to expand all the tabs in an existing code base so
>  as to bring it into compliance with a "no tabs" rule.)
> 
>                            Atlant
> 
> -----Original Message-----
> From: linux-mtd-bounces@lists.infradead.org [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of Artem Bityutskiy
> Sent: Friday, May 13, 2011 03:59
> To: Matthew L. Creech
> Cc: linux-mtd@lists.infradead.org
> Subject: Re: [PATCH 2/2] UBIFS: fix-up free space on mount if flag is set
> 
> On Thu, 2011-05-12 at 14:09 +0300, Artem Bityutskiy wrote:
>> On Fri, 2011-05-06 at 18:58 -0400, Matthew L. Creech wrote:
>>> +   if (c->space_fixup) {
>>> +           err = ubifs_fixup_free_space(c);
>>> +           if (err)
>>> +                   goto out;
>>> +   }
>>> +
>>>    dbg_gen("re-mounted read-write");
>>>    c->remounting_rw = 0;
>>>    err = dbg_check_space_info(c);
>> 
>> Note, I've moved the 'ubifs_fixup_free_space()' a bit down to make sure
>> we first print "re-mounted" and "deferred recovery completed" messages,
>> and then start fixing up. I think this is a bit neater.
> 
> I do not know if that was me or you, but I've found out that the a lot
> of code uses spaces for indentation instead of tabs, and checkpatch.pl
> complains:
> 
> WARNING: please, no spaces at the start of a line
> #119: FILE: fs/ubifs/sb.c:746:
> +                       err = PTR_ERR(lprops);$
> 
> ERROR: code indent should use tabs where possible
> #120: FILE: fs/ubifs/sb.c:747:
> +                       goto out;$
> 
> .... a lot of this ....
> 
> But I've just fixed this.
> 
> --
> Best Regards,
> Artem Bityutskiy (Артём Битюцкий)
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message.
> 
> Thank you.
> 
> Please consider the environment before printing this email.
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Artem Bityutskiy May 13, 2011, 12:29 p.m. UTC | #6
Hi,

On Fri, 2011-05-13 at 06:59 -0400, Atlant Schmidt wrote:
> Artem:
> 
>   I don't know what the coding standards are like on
>   your project,

The kenrel standard, described in Documentation/CodingStyle

>  but in very much of the C/C++ developer
>   world, <tab>s are frowned upon in code as being fragile,
>   primarily because there are varying standards for just
>   how wide a <tab> should be.

In the kernel we just stand that it must be 8 spaces.

>  By default, they're 8 spaces,
>   but I've worked at several shops where they are only
>   4 spaces and one shop where they are deemed to be 3!

Well, there were many flamewars about this, but we just assume that
anyone looking at the kernel source code has to setup their editors to
have tab=8 spaces.

>   Because of this, many coding standards (such as the
>   WebKit coding standards and my current employer's
>   standard) outright ban <tab> characters in code and
>   I'd recommend you do so as well, no matter what the
>   current scripts enforce. (It's certainly easy enough
>   to expand all the tabs in an existing code base so
>   as to bring it into compliance with a "no tabs" rule.)

Well, UBIFS is part of the kernel and it follows the kernel coding
style.
Artem Bityutskiy May 13, 2011, 12:34 p.m. UTC | #7
On Fri, 2011-05-13 at 15:29 +0300, Artem Bityutskiy wrote:
> Well, UBIFS is part of the kernel and it follows the kernel coding
> style.

Note, in this case I'm talking only about tabs as indentation, like

<tab>if (a)
<tab><tab>if (b)
<tab><tab><tab>function();

The other thing is tabs as "dynamic spaces", like:

int a<tab>=0;
int aa<tab>=0;

Some people use tabs as "dynamic spaces" because many editors will align
things. I personally do not appreciate this at all, and the kernel
Coding style does not require this. I only use tabs for indentation at
the beginning of the line.
diff mbox

Patch

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 17b92af..59e8858 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1396,6 +1396,12 @@  static int mount_ubifs(struct ubifs_info *c)
 	} else
 		ubifs_assert(c->lst.taken_empty_lebs > 0);
 
+	if (!c->ro_mount && c->space_fixup) {
+		err = ubifs_fixup_free_space(c);
+		if (err)
+			goto out_infos;
+	}
+
 	err = dbg_check_filesystem(c);
 	if (err)
 		goto out_infos;
@@ -1677,6 +1683,12 @@  static int ubifs_remount_rw(struct ubifs_info *c)
 		ubifs_msg("deferred recovery completed");
 	}
 
+	if (c->space_fixup) {
+		err = ubifs_fixup_free_space(c);
+		if (err)
+			goto out;
+	}
+
 	dbg_gen("re-mounted read-write");
 	c->remounting_rw = 0;
 	err = dbg_check_space_info(c);