Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Joe Perches]

On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
>  - There's no warning for the first paragraph of section 6:
> 
> 6) Functions
> 
> 
> Functions should be short and sweet, and do just one thing.  They should
> fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
> as we all know), and do one thing and do that well.
> 
>I'm not expecting you to be able to write a perl script that checks
>the first line, but we have way too many 200-plus line functions in
>the kernel.  I'd like a warning on anything over 200 lines (a factor
>of 4 over Linus's stated goal).

Perhaps something like this?

(very very lightly tested, more testing appreciated)
---
 scripts/checkpatch.pl | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4306b7616cdd..523aead40b87 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -59,6 +59,7 @@ my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
 my $color = "auto";
 my $allow_c99_comments = 1;
+my $max_function_length = 200;
 
 sub help {
my ($exitcode) = @_;
@@ -2202,6 +2203,7 @@ sub process {
my $realcnt = 0;
my $here = '';
my $context_function;   #undef'd unless there's a known function
+   my $context_function_linenum;
my $in_comment = 0;
my $comment_edge = 0;
my $first_line = 0;
@@ -2341,6 +2343,7 @@ sub process {
} else {
undef $context_function;
}
+   undef $context_function_linenum;
next;
 
 # track the line number as we move through the hunk, note that
@@ -3200,11 +3203,18 @@ sub process {
if ($sline =~ /^\+\{\s*$/ &&
$prevline =~ 
/^\+(?:(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*)?($Ident)\(/) {
$context_function = $1;
+   $context_function_linenum = $realline;
}
 
 # check if this appears to be the end of function declaration
if ($sline =~ /^\+\}\s*$/) {
+   if (defined($context_function_linenum) &&
+   ($realline - $context_function_linenum) > 
$max_function_length) {
+   WARN("LONG_FUNCTION",
+"'$context_function' function definition 
is " . ($realline - $context_function_linenum) . " lines, perhaps refactor\n" . 
$herecurr);
+   }
undef $context_function;
+   undef $context_function_linenum;
}
 
 # check indentation of any line with a bare else
@@ -5983,6 +5993,7 @@ sub process {
defined $stat &&
$stat =~ 
/^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) {
$context_function = $1;
+   $context_function_linenum = $realline;
 
 # check for multiline function definition with misplaced open brace
my $ok = 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Joe Perches]

On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> > Completely reasonable.  Thanks.
> 
> If we're doing "completely reasonable" complaints, then ...
> 
>  - I don't understand why plain 'unsigned' is deemed bad.

That was a David Miller preference.

>  - The rule about all function parameters in prototypes having a name
>doesn't make sense.  Example:
> 
> int ida_get_new_above(struct ida *ida, int starting_id, int *p_id);

Improvements to regex welcomed.

>  - Forcing a blank line after variable declarations sometimes makes for
>some weird-looking code.

True.  I don't care for this one myself.
>Constructively, I think this warning can be suppressed for blocks
>that are under, say, 8 lines.

Not easy to do as checkpatch works on patches.

> 6) Functions
> 
> 
> Functions should be short and sweet, and do just one thing.  They should
> fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
> as we all know), and do one thing and do that well.
> 
>I'm not expecting you to be able to write a perl script that checks
>the first line, but we have way too many 200-plus line functions in
>the kernel.  I'd like a warning on anything over 200 lines (a factor
>of 4 over Linus's stated goal).

Maybe reasonable.
Some declaration blocks for things like:

void foo(void)
{
static const struct foobar array[] = {
{ long count of lines... };
[body]
}

might make that warning unreasonable though.

>  - I don't understand the error for xa_head here:
> 
> struct xarray {
> spinlock_t  xa_lock;
> gfp_t   xa_flags;
> void __rcu *xa_head;
> };
> 
>Do people really think that:
> 
> struct xarray {
> spinlock_t  xa_lock;
> gfp_t   xa_flags;
> void __rcu*xa_head;
> };
> 
>is more aesthetically pleasing?  And not just that, but it's an *error*
>so the former is *RIGHT* and this is *WRONG*.  And not just a matter
>of taste?

No opinion really.
That's from Andy Whitcroft's original implementation.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Joe Perches]

On Tue, 2017-12-12 at 08:43 +1100, Dave Chinner wrote:
> On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
> > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> > >   1. Using lockdep_set_novalidate_class() for anything other
> > >   than device->mutex will throw checkpatch warnings. Nice. (*)
> > []
> > > (*) checkpatch.pl is considered mostly harmful round here, too,
> > > but that's another rant
> > 
> > How so?
> 
> Short story is that it barfs all over the slightly non-standard
> coding style used in XFS.
[]
> This sort of stuff is just lowest-common-denominator noise - great
> for new code and/or inexperienced developers, but not for working
> with large bodies of existing code with slightly non-standard
> conventions.

Completely reasonable.  Thanks.

Do you get many checkpatch submitters for fs/xfs?

If so, could probably do something about adding
a checkpatch file flag to the directory or equivalent.

Maybe add something like:

fs/xfs/.checkpatch

where the contents turn off most everything
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Joe Perches]

On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
>   1. Using lockdep_set_novalidate_class() for anything other
>   than device->mutex will throw checkpatch warnings. Nice. (*)
[]
> (*) checkpatch.pl is considered mostly harmful round here, too,
> but that's another rant

How so?

> (**) the frequent occurrence of "core code/devs aren't held to the
> same rules/standard as everyone else" is another rant I have stored
> up for a rainy day.

Yeah.  I wouldn't mind reading that one...

Rainy season is starting right about now here too.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation

[Joe Perches]

On Wed, 2017-09-13 at 13:02 +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais 

I think the changelog for this series of conversions
should show that you've validated the change by
inspecting the return call chain at each modified line.

Also, it seems you've cc'd the same mailing lists for
all of the patches modified by this series.

It would be better to individually address each patch
in the series only cc'ing the appropriate maintainers
and mailing lists.

A cover letter would be good too.

> ---
>  arch/powerpc/platforms/cell/spider-pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/spider-pci.c 
> b/arch/powerpc/platforms/cell/spider-pci.c
> index d1e61e2..82aa3f7 100644
> --- a/arch/powerpc/platforms/cell/spider-pci.c
> +++ b/arch/powerpc/platforms/cell/spider-pci.c
> @@ -106,7 +106,7 @@ static int __init spiderpci_pci_setup_chip(struct 
> pci_controller *phb,
>   dummy_page_va = kmalloc(PAGE_SIZE, GFP_KERNEL);
>   if (!dummy_page_va) {
>   pr_err("SPIDERPCI-IOWA:Alloc dummy_page_va failed.\n");
> - return -1;
> + return -ENOMEM;
>   }
>  
>   dummy_page_da = dma_map_single(phb->parent, dummy_page_va,
> @@ -137,7 +137,7 @@ int __init spiderpci_iowa_init(struct iowa_bus *bus, void 
> *data)
>   if (!priv) {
>   pr_err("SPIDERPCI-IOWA:"
>  "Can't allocate struct spiderpci_iowa_private");
> - return -1;
> + return -ENOMEM;
>   }
>  
>   if (of_address_to_resource(np, 0, )) {
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: ctree: added lines after variable declarations

[Joe Perches]

On Mon, 2016-02-22 at 01:31 +0100, Philippe Loctaux wrote:
> Hi,
> I'm really sorry, but I don't understand what you're trying to mean.
> Could you simplify your sentence please (since I'm not english
> native)?
> I'd really apreciate that, thanks :)

Please do not put your reply at the top of the email. (top-post)

This was the only instance I found where your modifications
were inappropriate (just here)

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: compression: added line after variable declaration

[Joe Perches]

On Mon, 2016-02-22 at 00:26 +0100, Philippe Loctaux wrote:
> Added line after variable declaration, fixing checkpatch warning.
[]
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
[]
> @@ -522,6 +522,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>  
>   if (zero_offset) {
>   int zeros;
> +
>   zeros = PAGE_CACHE_SIZE - zero_offset;
>   userpage = kmap_atomic(page);
>   memset(userpage + zero_offset, 0, zeros);

This zeros temporary is used once.
Perhaps it should just be removed instead.

Maybe the userpage declaration should move too.

Something like:
---
 fs/btrfs/compression.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 3346cd8..be41f83 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -517,14 +517,13 @@ static noinline int add_ra_bio_pages(struct inode *inode,
    free_extent_map(em);
 
    if (page->index == end_index) {
-   char *userpage;
    size_t zero_offset = isize & (PAGE_CACHE_SIZE - 1);
 
    if (zero_offset) {
-   int zeros;
-   zeros = PAGE_CACHE_SIZE - zero_offset;
-   userpage = kmap_atomic(page);
-   memset(userpage + zero_offset, 0, zeros);
+   char *userpage = kmap_atomic(page);
+
+   memset(userpage + zero_offset, 0,
+      PAGE_CACHE_SIZE - zero_offset);
    flush_dcache_page(page);
    kunmap_atomic(userpage);
    }

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: ctree: added lines after variable declarations

[Joe Perches]

On Mon, 2016-02-22 at 01:01 +0100, Philippe Loctaux wrote:
> Is there no need of additional blank line here particulary
> or in all lines that I changed?

Please don't top post and just here.
> On Sun, Feb 21, 2016 at 03:53:04PM -0800, Joe Perches wrote:
> > On Mon, 2016-02-22 at 00:46 +0100, Philippe Loctaux wrote:
> > > Added lines after variable declarations, fixing 22 checkpatch warnings.
> > []
> > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> > []
> > > @@ -4592,6 +4610,7 @@ void btrfs_truncate_item(struct btrfs_root *root, 
> > > struct btrfs_path *path,
> > >     data_end, old_data_start + new_size - data_end);
> > >   } else {
> > >   struct btrfs_disk_key disk_key;
> > > +
> > >   u64 offset;
> > 
> > Overzealous here.
> > No additional blank line required.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: ctree: added lines after variable declarations

[Joe Perches]

On Mon, 2016-02-22 at 00:46 +0100, Philippe Loctaux wrote:
> Added lines after variable declarations, fixing 22 checkpatch warnings.
[]
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
[]
> @@ -4592,6 +4610,7 @@ void btrfs_truncate_item(struct btrfs_root *root, 
> struct btrfs_path *path,
>     data_end, old_data_start + new_size - data_end);
>   } else {
>   struct btrfs_disk_key disk_key;
> +
>   u64 offset;

Overzealous here.
No additional blank line required.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: backref: Fixed checkpatch warning of over 80

[Joe Perches]

On Sat, 2016-02-20 at 18:57 -0600, Simon Quigley wrote:
> Better?

No, not really.

The alignment should be to the open parenthesis
as I wrote in the first reply.

> On 02/20/2016 06:56 PM, Simon Quigley wrote:
> > checkpatch.pl reported a warning of over 80 characters on line 1833
> > 
> > Adjusted to put  and  on a different line
> > 
> > Signed-off-by: Simon Quigley 
> > ---
> >   fs/btrfs/backref.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index f6dac40..956fffa 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -1830,8 +1830,8 @@ static int iterate_inode_extrefs(u64 inum,
> > struct btrfs_root *fs_root,
> >     unsigned long ptr;
> > 
> >     while (1) {
> > -   ret = btrfs_find_one_extref(fs_root, inum, offset,
> > path, ,
> > -   );
> > +   ret = btrfs_find_one_extref(fs_root, inum, offset,
> > path,
> > +   ,
> > );
> >     if (ret < 0)
> >     break;
> >     if (ret) {
> > 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: backref: Fixed checkpatch warning of over 80 characters

[Joe Perches]

On Sat, 2016-02-20 at 12:17 -0600, Simon Quigley wrote:
> checkpatch.pl reported a warning of over 80 characters on line 1833
[]
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
[]
> @@ -1830,7 +1830,11 @@ static int iterate_inode_extrefs(u64 inum, struct 
> btrfs_root *fs_root,
>   unsigned long ptr;
>  
>   while (1) {
> - ret = btrfs_find_one_extref(fs_root, inum, offset, path, 
> ,
> + ret = btrfs_find_one_extref(fs_root,
> + inum,
> + offset,
> + path,
> + ,
>   );

I think this is poor because all the arguments aren't aligned.

It'd be nicer like:

ret = btrfs_find_one_extref(fs_root, inum, offset, path,
, );

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] Move BTRFS RCU string to common library

[Joe Perches]

On Sat, 2014-11-08 at 00:46 -0800, Omar Sandoval wrote:
 The RCU-friendly string API used internally by BTRFS is generic enough for
 common use. This doesn't add any new functionality, but instead just moves the
 code and documents the existing API.

Some more trivia, can be updated later if desired.

 diff --git a/include/linux/rcustring.h b/include/linux/rcustring.h
 new file mode 100644
[]
 +
 +/**
 + * printk_in_rcu() - printk in an RCU read-side critical section

It'd be nice to show the arguments for kernel-doc here

 + */
 +#define printk_in_rcu(fmt, ...)  \
 + do {\
 + rcu_read_lock();\
 + printk(fmt, __VA_ARGS__);   \

This should use ##__VA_ARGS__ to allow the
compiler to elide the comma when using

printk_in_rcu(Hello World!);

 + rcu_read_unlock();  \
 + } while (0)
 +
 +/**
 + * printk_ratelimited_in_rcu() - printk_ratelimited in an RCU read-side 
 critical
 + * section
 + */
 +#define printk_ratelimited_in_rcu(fmt, ...)  \
 + do {\
 + rcu_read_lock();\
 + printk_ratelimited(fmt, __VA_ARGS__);   \
 + rcu_read_unlock();  \
 + } while (0)
 +
 +#endif

Here too

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] Move BTRFS RCU string to common library

[Joe Perches]

On Fri, 2014-11-07 at 15:21 -0500, Chris Mason wrote:
 
 On Fri, Nov 7, 2014 at 3:17 PM, Omar Sandoval osan...@osandov.com 
 wrote:
  The RCU-friendly string API used internally by BTRFS is generic 
  enough for
  common use. This doesn't add any new functionality, but instead just 
  moves the
  code and documents the existing API.
  
  Reviewed-by: Josh Triplett j...@joshtriplett.org
  Acked-by: Paul E. McKenney paul...@linux.vnet.ibm.com
  Signed-off-by: Omar Sandoval osan...@osandov.com
  ---
  Chris, could you take a look at this again? It's just been rebased 
  and should
  be good to go. Thanks!
 
 Thanks Omar, I'm pulling this into a branch for the next merge window.  
 Looks good to me.

rcustring.h should probably #include any #include dependencies
it has.  types.h, printk.h, etc...


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] Return a value from printk_ratelimited

[Joe Perches]

On Fri, 2014-09-26 at 22:36 -0700, Omar Sandoval wrote:
 printk returns an integer; there's no reason for printk_ratelimited to swallow
 it.
 
 Signed-off-by: Omar Sandoval osan...@osandov.com
 Acked-by: Paul E. McKenney paul...@linux.vnet.ibm.com
 ---

I'd prefer to keep it the way it is actually.

I've submitted several patches to convert the int return
to void for printk derived functions recently.


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] Return a value from printk_ratelimited

[Joe Perches]

On Sun, 2014-09-21 at 06:25 -0700, Paul E. McKenney wrote:
 On Fri, Sep 19, 2014 at 11:15:53AM -0700, Joe Perches wrote:
  On Fri, 2014-09-19 at 13:21 -0400, Steven Rostedt wrote:
   On Fri, 19 Sep 2014 02:01:29 -0700
   Omar Sandoval osan...@osandov.com wrote:
   
printk returns an integer; there's no reason for printk_ratelimited to 
swallow
it.
  
  Except for the lack of usefulness of the return value itself.
  See: https://lkml.org/lkml/2009/10/7/275
 
 When printk()'s return value is changed to void, then yes, we should
 clearly change this code to match that.
 
 So, I have to ask...  What happened to the patch later in that series
 that was to remove the uses of the printk() return value?

I don't know.

Last I recall via searching emails, Alan Jenkins was going to do
something with it. (I've added his old email to this reply, but
I doubt still works)

I remember checking whether or not the removing the return value
reduced the code size on x86 (it did not), and forgot about it.

I don't know if removing the printk return value reduces overall
image size in any arch, so I didn't pursue it.

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] Return a value from printk_ratelimited

[Joe Perches]

On Fri, 2014-09-19 at 13:21 -0400, Steven Rostedt wrote:
 On Fri, 19 Sep 2014 02:01:29 -0700
 Omar Sandoval osan...@osandov.com wrote:
 
  printk returns an integer; there's no reason for printk_ratelimited to 
  swallow
  it.

Except for the lack of usefulness of the return value itself.
See: https://lkml.org/lkml/2009/10/7/275


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

[Joe Perches]

On Wed, 2013-08-14 at 10:56 +0200, Geert Uytterhoeven wrote:
 These bring in the 64-bit divisor from somewhere else, so they're less
 trivial to fix.

Using div64_u64 or div64_s64 could fix it.
Maybe that could be added to do_div too.

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide (was: Re: Linux 3.10 problem reports (yes, plural))

[Joe Perches]

On Tue, 2013-07-30 at 13:13 -0400, Josef Bacik wrote:
 I've looked at all the places we do divides in this function and it
 doesn't look like we're doing this anywhere but I could be blind,

do_div seems a likely suspect...

/*
 * stripe_nr counts the total number of stripes we have to stride
 * to get to this block
 */
do_div(stripe_nr, stripe_len);


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

[Joe Perches]

On Tue, 2013-07-30 at 16:40 -0400, Josef Bacik wrote:
 So stripe_len shouldn't be 0, if it is you have bigger problems :).  Is this a
 corrupt fs or something?  If there was some sort of corruption that occured 
 then
 I suppose stripe_len could be 0 and we'd need to catch that somewhere higher 
 up
 the stack and error out.  Is there a way you could check and see if that's the
 case?  Thanks,

Maybe use a temporary check in do_div
Something like this maybe. (uncompiled/untested)
---
 include/asm-generic/div64.h | 43 +--
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
index 8f4e319..cce75fe 100644
--- a/include/asm-generic/div64.h
+++ b/include/asm-generic/div64.h
@@ -19,16 +19,25 @@
 
 #include linux/types.h
 #include linux/compiler.h
+#include linux/bug.h
+#include linux/printk.h
 
 #if BITS_PER_LONG == 64
 
-# define do_div(n,base) ({ \
+# define do_div(n, base)   \
+({ \
uint32_t __base = (base);   \
uint32_t __rem; \
-   __rem = ((uint64_t)(n)) % __base;   \
-   (n) = ((uint64_t)(n)) / __base; \
+   if (__base == 0) {  \
+   WARN(1, Attempted division by 0\n);   \
+   dump_stack();   \
+   __rem = 0;  \
+   } else {\
+   __rem = ((uint64_t)(n)) % __base;   \
+   (n) = ((uint64_t)(n)) / __base; \
+   }   \
__rem;  \
- })
+})
 
 #elif BITS_PER_LONG == 32
 
@@ -37,16 +46,22 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t 
divisor);
 /* The unnecessary pointer compare is there
  * to check for type safety (n must be 64bit)
  */
-# define do_div(n,base) ({ \
-   uint32_t __base = (base);   \
-   uint32_t __rem; \
-   (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
-   if (likely(((n)  32) == 0)) { \
-   __rem = (uint32_t)(n) % __base; \
-   (n) = (uint32_t)(n) / __base;   \
-   } else  \
-   __rem = __div64_32((n), __base);   \
-   __rem;  \
+# define do_div(n, base)   \
+({ \
+   uint32_t __base = (base);   \
+   uint32_t __rem; \
+   (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
+   if (__base == 0) {  \
+   WARN(1, Attempted division by 0\n);   \
+   dump_stack();   \
+   __rem = 0;  \
+   } else if (likely(((n)  32) == 0)) {  \
+   __rem = (uint32_t)(n) % __base; \
+   (n) = (uint32_t)(n) / __base;   \
+   } else {\
+   __rem = __div64_32((n), __base);   \
+   }   \
+   __rem;  \
  })
 
 #else /* BITS_PER_LONG == ?? */


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [trivial PATCH] treewide: Add __GFP_NOWARN to k.alloc calls with v.alloc fallbacks

[Joe Perches]

On Wed, 2013-06-19 at 12:15 -0700, Joe Perches wrote:
 Don't emit OOM warnings when k.alloc calls fail when
 there there is a v.alloc immediately afterwards.
 
 Converted a kmalloc/vmalloc with memset to kzalloc/vzalloc.

Hey Jiri.

What's your schedule for accepting or rejecting
these sorts of patches?

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[trivial PATCH] treewide: Add __GFP_NOWARN to k.alloc calls with v.alloc fallbacks

[Joe Perches]

Don't emit OOM warnings when k.alloc calls fail when
there there is a v.alloc immediately afterwards.

Converted a kmalloc/vmalloc with memset to kzalloc/vzalloc.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/block/drbd/drbd_bitmap.c   | 2 +-
 drivers/infiniband/hw/ehca/ipz_pt_fn.c | 3 ++-
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 2 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 2 +-
 drivers/scsi/cxgbi/libcxgbi.h  | 8 
 fs/btrfs/send.c| 2 +-
 fs/ext4/super.c| 4 ++--
 fs/gfs2/dir.c  | 2 +-
 net/sched/sch_choke.c  | 3 ++-
 9 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 64fbb83..b12c11e 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -393,7 +393,7 @@ static struct page **bm_realloc_pages(struct drbd_bitmap 
*b, unsigned long want)
 * we must not block on IO to ourselves.
 * Context is receiver thread or dmsetup. */
bytes = sizeof(struct page *)*want;
-   new_pages = kzalloc(bytes, GFP_NOIO);
+   new_pages = kzalloc(bytes, GFP_NOIO | __GFP_NOWARN);
if (!new_pages) {
new_pages = __vmalloc(bytes,
GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO,
diff --git a/drivers/infiniband/hw/ehca/ipz_pt_fn.c 
b/drivers/infiniband/hw/ehca/ipz_pt_fn.c
index 62c71fa..8d59451 100644
--- a/drivers/infiniband/hw/ehca/ipz_pt_fn.c
+++ b/drivers/infiniband/hw/ehca/ipz_pt_fn.c
@@ -222,7 +222,8 @@ int ipz_queue_ctor(struct ehca_pd *pd, struct ipz_queue 
*queue,
queue-small_page = NULL;
 
/* allocate queue page pointers */
-   queue-queue_pages = kzalloc(nr_of_pages * sizeof(void *), GFP_KERNEL);
+   queue-queue_pages = kzalloc(nr_of_pages * sizeof(void *),
+GFP_KERNEL | __GFP_NOWARN);
if (!queue-queue_pages) {
queue-queue_pages = vzalloc(nr_of_pages * sizeof(void *));
if (!queue-queue_pages) {
diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c 
b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
index 4058b85..76ae0999 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
@@ -1157,7 +1157,7 @@ static void cxgb_redirect(struct dst_entry *old, struct 
dst_entry *new,
  */
 void *cxgb_alloc_mem(unsigned long size)
 {
-   void *p = kzalloc(size, GFP_KERNEL);
+   void *p = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
 
if (!p)
p = vzalloc(size);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 5a3256b..5d5f268 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -1133,7 +1133,7 @@ out:  release_firmware(fw);
  */
 void *t4_alloc_mem(size_t size)
 {
-   void *p = kzalloc(size, GFP_KERNEL);
+   void *p = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
 
if (!p)
p = vzalloc(size);
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 80fa99b..8135f04 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -658,11 +658,11 @@ static inline u32 cxgbi_tag_nonrsvd_bits(struct 
cxgbi_tag_format *tformat,
 static inline void *cxgbi_alloc_big_mem(unsigned int size,
gfp_t gfp)
 {
-   void *p = kmalloc(size, gfp);
+   void *p = kzalloc(size, gfp | __GFP_NOWARN);
+
if (!p)
-   p = vmalloc(size);
-   if (p)
-   memset(p, 0, size);
+   p = vzalloc(size);
+
return p;
 }
 
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index ff40f1c..f7499ed 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -219,7 +219,7 @@ static int fs_path_ensure_buf(struct fs_path *p, int len)
len = PAGE_ALIGN(len);
 
if (p-buf == p-inline_buf) {
-   tmp_buf = kmalloc(len, GFP_NOFS);
+   tmp_buf = kmalloc(len, GFP_NOFS | __GFP_NOWARN);
if (!tmp_buf) {
tmp_buf = vmalloc(len);
if (!tmp_buf)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 85b3dd6..0f94f55 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -162,7 +162,7 @@ void *ext4_kvmalloc(size_t size, gfp_t flags)
 {
void *ret;
 
-   ret = kmalloc(size, flags);
+   ret = kmalloc(size, flags | __GFP_NOWARN);
if (!ret)
ret = __vmalloc(size, flags, PAGE_KERNEL);
return ret;
@@ -172,7 +172,7 @@ void *ext4_kvzalloc(size_t size, gfp_t flags)
 {
void *ret;
 
-   ret = kzalloc(size, flags);
+   ret = kzalloc(size

[PATCH 4/8] btrfs: Use printk_get_level and printk_skip_level, add __printf, fix fallout

[Joe Perches]

Use the generic printk_get_level function to search a message
for a kern_level.  Add __printf to verify format and arguments.
Fix a few messages that had mismatches in format and arguments.
Add #ifdef CONFIG_PRINTK blocks to shrink the object size a bit
when not using printk.

Signed-off-by: Joe Perches j...@perches.com
---
 fs/btrfs/ctree.h  |   13 +
 fs/btrfs/disk-io.c|2 +-
 fs/btrfs/relocation.c |2 +-
 fs/btrfs/super.c  |   41 +++--
 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0236d03..590b519 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3046,10 +3046,22 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char 
*buffer, size_t size);
 /* super.c */
 int btrfs_parse_options(struct btrfs_root *root, char *options);
 int btrfs_sync_fs(struct super_block *sb, int wait);
+
+#ifdef CONFIG_PRINTK
+__printf(2, 3)
 void btrfs_printk(struct btrfs_fs_info *fs_info, const char *fmt, ...);
+#else
+static inline __printf(2, 3)
+void btrfs_printk(struct btrfs_fs_info *fs_info, const char *fmt, ...)
+{
+}
+#endif
+
+__printf(5, 6)
 void __btrfs_std_error(struct btrfs_fs_info *fs_info, const char *function,
 unsigned int line, int errno, const char *fmt, ...);
 
+
 void __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
   struct btrfs_root *root, const char *function,
   unsigned int line, int errno);
@@ -3073,6 +3085,7 @@ do {  
\
  (errno), fmt, ##args);\
 } while (0)
 
+__printf(5, 6)
 void __btrfs_panic(struct btrfs_fs_info *fs_info, const char *function,
   unsigned int line, int errno, const char *fmt, ...);
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 7ae51de..f02bba5 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1113,7 +1113,7 @@ void clean_tree_block(struct btrfs_trans_handle *trans, 
struct btrfs_root *root,
spin_unlock(root-fs_info-delalloc_lock);
btrfs_panic(root-fs_info, -EOVERFLOW,
  Can't clear %lu bytes from 
-  dirty_mdatadata_bytes (%lu),
+  dirty_mdatadata_bytes (%llu),
  buf-len,
  root-fs_info-dirty_metadata_bytes);
}
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 646ee21..790f492 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1242,7 +1242,7 @@ static int __must_check __add_reloc_root(struct 
btrfs_root *root)
kfree(node);
btrfs_panic(root-fs_info, -EEXIST, Duplicate root found 
for start=%llu while inserting into relocation 
-   tree\n);
+   tree\n, node-bytenr);
}
 
list_add_tail(root-root_list, rc-reloc_roots);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 96eb9fe..a84529d 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -124,6 +124,7 @@ static void btrfs_handle_error(struct btrfs_fs_info 
*fs_info)
}
 }
 
+#ifdef CONFIG_PRINTK
 /*
  * __btrfs_std_error decodes expected errors from the caller and
  * invokes the approciate error response.
@@ -166,7 +167,7 @@ void __btrfs_std_error(struct btrfs_fs_info *fs_info, const 
char *function,
va_end(args);
 }
 
-const char *logtypes[] = {
+static const char *logtypes[] = {
emergency,
alert,
critical,
@@ -184,22 +185,50 @@ void btrfs_printk(struct btrfs_fs_info *fs_info, const 
char *fmt, ...)
struct va_format vaf;
va_list args;
const char *type = logtypes[4];
+   int kern_level;
 
va_start(args, fmt);
 
-   if (fmt[0] == ''  isdigit(fmt[1])  fmt[2] == '') {
-   memcpy(lvl, fmt, 3);
-   lvl[3] = '\0';
-   fmt += 3;
-   type = logtypes[fmt[1] - '0'];
+   kern_level = printk_get_level(fmt);
+   if (kern_level) {
+   size_t size = printk_skip_level(fmt)  - fmt;
+   memcpy(lvl, fmt,  size);
+   lvl[size] = '\0';
+   fmt += size;
+   type = logtypes[kern_level - '0'];
} else
*lvl = '\0';
 
vaf.fmt = fmt;
vaf.va = args;
+
printk(%sBTRFS %s (device %s): %pV, lvl, type, sb-s_id, vaf);
+
+   va_end(args);
 }
 
+#else
+
+void __btrfs_std_error(struct btrfs_fs_info *fs_info, const char *function,
+  unsigned int line, int errno, const char *fmt, ...)
+{
+   struct super_block *sb = fs_info-sb;
+
+   /*
+* Special case: if the error is EROFS, and we're already

[PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

KERN_LEVEL currently takes up 3 bytes.
Shrink the kernel size by using an ASCII SOH and then the level byte.
Remove the need for KERN_CONT.
Convert directly embedded uses of . to KERN_LEVEL

Joe Perches (8):
  printk: Add generic functions to find KERN_LEVEL headers
  printk: Add kern_levels.h to make KERN_LEVEL available for asm use
  arch: Remove direct definitions of KERN_LEVEL uses
  btrfs: Use printk_get_level and printk_skip_level, add __printf, fix fallout
  sound: Use printk_get_level and printk_skip_level
  staging: wlags49_h2: Remove direct declarations of KERN_LEVEL prefixes
  printk: Convert the format for KERN_LEVEL to a 2 byte pattern
  printk: Remove the now unnecessary C annotation for KERN_CONT

 arch/arm/lib/io-acorn.S  |3 +-
 arch/arm/vfp/vfphw.S |7 +++--
 arch/frv/kernel/kernel_thread.S  |2 +-
 drivers/staging/wlags49_h2/hcf.c |8 +++---
 drivers/staging/wlags49_h2/wl_main.c |4 +-
 fs/btrfs/ctree.h |   13 ++
 fs/btrfs/disk-io.c   |2 +-
 fs/btrfs/relocation.c|2 +-
 fs/btrfs/super.c |   41 +-
 include/linux/kern_levels.h  |   25 
 include/linux/printk.h   |   41 +++--
 kernel/printk.c  |   14 +++
 sound/core/misc.c|   13 +++---
 13 files changed, 130 insertions(+), 45 deletions(-)
 create mode 100644 include/linux/kern_levels.h

-- 
1.7.8.111.gad25c.dirty

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Tue, 2012-06-05 at 14:28 -0700, Andrew Morton wrote:
 Unfortunately the n thing is part of the kernel ABI:
 
   echo 4foo  /dev/kmsg

Which works the same way it did before.


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Tue, 2012-06-05 at 15:17 -0700, Andrew Morton wrote:
 On Tue, 05 Jun 2012 15:11:43 -0700
 Joe Perches j...@perches.com wrote:
 
  On Tue, 2012-06-05 at 14:28 -0700, Andrew Morton wrote:
   Unfortunately the n thing is part of the kernel ABI:
   
 echo 4foo  /dev/kmsg
  
  Which works the same way it did before.
 
 I didn't say it didn't.
 
 What I did say is that echo \0014/dev/kmsg will subvert the intent
 of the new logging code.  Or might.  But you just ignored all that,
 forcing me to repeat myself, irritatedly.

It works the same way before and after the patch.

Any write to /dev/kmsg without a KERN_LEVEL
emits at (1  3) + KERN_DEFAULT.

Writes with n values = 8 are emitted at
that level.


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Tue, 2012-06-05 at 16:29 -0700, Andrew Morton wrote:
 What about writes starting with \001n?  AFACIT, that will be stripped
 away and the printk level will be altered.  This is new behavior.

Nope.

# echo \001Hello Andrew  /dev/kmsg
/dev/kmsg has
12,774,2462339252;\001Hello Andrew

12 is 8 + KERN_DEFAULT

# echo 1Hello Andrew  /dev/kmsg
/dev/kmsg has:
9,775,2516023444;Hello Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Wed, 2012-06-06 at 01:39 +0200, Kay Sievers wrote:
 On Wed, Jun 6, 2012 at 1:35 AM, Joe Perches j...@perches.com wrote:
  On Tue, 2012-06-05 at 16:29 -0700, Andrew Morton wrote:
  What about writes starting with \001n?  AFACIT, that will be stripped
  away and the printk level will be altered.  This is new behavior.
 
  Nope.
 
  # echo \001Hello Andrew  /dev/kmsg
  /dev/kmsg has
  12,774,2462339252;\001Hello Andrew
 
 Try echo -e? The stuff is copied verbatim otherwise.

# echo -e \001Hello Kay  /dev/kmsg
gives
12,776,3046752764;\x01Hello Kay


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Wed, 2012-06-06 at 01:39 +0200, Kay Sievers wrote:
 Try echo -e? The stuff is copied verbatim otherwise.

btw: I didn't change the devkmsg_writev function
which does the decoding of any n prefix for
printk_emit.


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Tue, 2012-06-05 at 16:58 -0700, Andrew Morton wrote:
  echo \0014Hello Joe  /dev/kmsg

# echo -e \x014Hello Me  /dev/kmsg
gives:
12,778,4057982669;Hello Me

#echo -e \x011Hello Me_2  /dev/kmsg
gives:
12,779,4140452093;Hello Me_2

I didn't change devkmsg_writev so the
original parsing style for . is
unchanged.

from printk.c:

static ssize_t devkmsg_writev(struct kiocb *iocb, const struct iovec *iv,
  unsigned long count, loff_t pos)
[...]
int level = default_message_loglevel;
[...]
if (line[0] == '') {
char *endp = NULL;

i = simple_strtoul(line+1, endp, 10);
if (endp  endp[0] == '') {
level = i  7;
if (i  3)
facility = i  3;
endp++;
len -= endp - line;
line = endp;
}
}
line[len] = '\0';

printk_emit(facility, level, NULL, 0, %s, line);
[]

level is what matters.

from dmesg -r

12[ 2462.339252] \001Hello Andrew
9[ 2516.023444] Hello Andrew
12[ 3046.752764] \x01Hello Kay
12[ 3940.871850] \x01Hello Kay
12[ 4057.982669] Hello Me
12[ 4140.452093] Hello Me_2


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Wed, 2012-06-06 at 02:13 +0200, Kay Sievers wrote:
 The question is what happens if you inject your new binary two-byte
 prefix, like:
   echo -e \x01\x02Hello  /dev/kmsg

It's not a 2 byte binary.
It's a leading ascii SOH and a standard ascii char
'0' ... '7' or 'd'.

#define KERN_EMERG  KERN_SOH 0/* system is unusable */
#define KERN_ALERT  KERN_SOH 1/* action must be taken immediately */
etc...

 And if that changes the log-level to 2 instead of the default 4?

No it doesn't.

It's not triggering that because devkmsg_writev does
prefix parsing only on the old n form.


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Wed, 2012-06-06 at 02:28 +0200, Kay Sievers wrote:
 On Wed, Jun 6, 2012 at 2:19 AM, Joe Perches j...@perches.com wrote:
  On Wed, 2012-06-06 at 02:13 +0200, Kay Sievers wrote:
  The question is what happens if you inject your new binary two-byte
  prefix, like:
echo -e \x01\x02Hello  /dev/kmsg
 
  It's not a 2 byte binary.
  It's a leading ascii SOH and a standard ascii char
  '0' ... '7' or 'd'.
 
  #define KERN_EMERG  KERN_SOH 0/* system is unusable */
  #define KERN_ALERT  KERN_SOH 1/* action must be taken immediately 
  */
  etc...
 
 Ok.
 
  And if that changes the log-level to 2 instead of the default 4?
 
  No it doesn't.
 
 So:
echo -e \x012Hello  /dev/kmsg
 is still level 4? Sounds all fine then.

Yes.
# echo -e \x012Hello again Kay  /dev/kmsg
gives:
12,780,6031964979;Hello again Kay

  It's not triggering that because devkmsg_writev does
  prefix parsing only on the old n form.
 
 Yeah, but printk_emit() will not try to parse it? I did not check, but
 with your change, the prefix parsing in printk_emit() is still skipped
 if a level is given as a parameter to printk_emit(), right?

If level is not -1, then whatever prefix level the
string has is ignored by vprintk_emit.

from vprintk_emit:

/* strip syslog prefix and extract log level or control flags */
kern_level = printk_get_level(text);
if (kern_level) {
const char *end_of_header = printk_skip_level(text);
switch (kern_level) {
case '0' ... '7':
if (level == -1)
level = kern_level - '0';
case 'd': /* Strip d KERN_DEFAULT, start new line */
plen = 0;
default:
if (!new_text_line) {
log_buf_emit_char('\n');
new_text_line = 1;
}
}
text_len -=  end_of_header - text;
text = (char *)end_of_header;
}

Only level == -1 will use the prefix level.

devkmsg_writev always passes a non -1 level.

cheers, Joe

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Tue, 2012-06-05 at 17:37 -0700, Andrew Morton wrote:
 On Tue, 05 Jun 2012 17:07:27 -0700 Joe Perches j...@perches.com wrote:
 
  On Tue, 2012-06-05 at 16:58 -0700, Andrew Morton wrote:
echo \0014Hello Joe  /dev/kmsg
  
  # echo -e \x014Hello Me  /dev/kmsg
  gives:
  12,778,4057982669;Hello Me
 
 That's changed behavior.

Which is an improvement too.
I very much doubt a single app will change
because of this.

 printk_emit() does parse the leading \0014, and then skips over it,
 removing it from the output stream.  printk_emit() then throws away the
 resulting level because devkmsg_writev() did not pass in level==-1.

I'm glad you know how it works now.

cheers, Joe


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rework KERN_LEVEL

[Joe Perches]

On Wed, 2012-06-06 at 03:10 +0200, Kay Sievers wrote:
 On Wed, Jun 6, 2012 at 2:46 AM, Andrew Morton a...@linux-foundation.org 
 wrote:
  On Tue, 05 Jun 2012 17:40:05 -0700 Joe Perches j...@perches.com wrote:
 
  On Tue, 2012-06-05 at 17:37 -0700, Andrew Morton wrote:
   On Tue, 05 Jun 2012 17:07:27 -0700 Joe Perches j...@perches.com wrote:
  
On Tue, 2012-06-05 at 16:58 -0700, Andrew Morton wrote:
  echo \0014Hello Joe  /dev/kmsg
   
# echo -e \x014Hello Me  /dev/kmsg
gives:
12,778,4057982669;Hello Me
  
   That's changed behavior.
 
  Which is an improvement too.
 
  No it isn't.  It exposes internal kernel implementation details in
  random weird inexplicable ways.  It doesn't seem at all important
  though.
 
  I very much doubt a single app will change
  because of this.
 
  I doubt it as well.
 
 Yeah, the value of injecting such binary data is kind of questionable. :)
 
 Joe, maybe you can change printk_emit() to skip the prefix
 detection/stripping if a prefix is already passed to the function?

Sure, it's pretty trivial.

Perhaps all binary data data should be elided.
Maybe print . for all non-printable ascii chars.


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 9/8] printk: Only look for prefix levels in kernel messages

[Joe Perches]

vprintk_emit prefix parsing should only be done for internal
kernel messages.  This allows existing behavior to be kept
in all cases.

Signed-off-by: Joe Perches j...@perches.com
---
 kernel/printk.c |   32 +---
 1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 5cd73f7..4e72c07 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1267,7 +1267,6 @@ asmlinkage int vprintk_emit(int facility, int level,
static char cont_buf[LOG_LINE_MAX];
static size_t cont_len;
static int cont_level;
-   int kern_level;
static struct task_struct *cont_task;
static char textbuf[LOG_LINE_MAX];
char *text = textbuf;
@@ -1329,21 +1328,24 @@ asmlinkage int vprintk_emit(int facility, int level,
newline = true;
}
 
-   /* strip syslog prefix and extract log level or control flags */
-   kern_level = printk_get_level(text);
-   if (kern_level) {
-   const char *end_of_header = printk_skip_level(text);
-   switch (kern_level) {
-   case '0' ... '7':
-   if (level == -1)
-   level = kern_level - '0';
-   case 'd':   /* KERN_DEFAULT */
-   prefix = true;
-   case 'c':   /* KERN_CONT */
-   break;
+   /* strip kernel syslog prefix and extract log level or control flags */
+   if (facility == 0) {
+   int kern_level = printk_get_level(text);
+
+   if (kern_level) {
+   const char *end_of_header = printk_skip_level(text);
+   switch (kern_level) {
+   case '0' ... '7':
+   if (level == -1)
+   level = kern_level - '0';
+   case 'd':   /* KERN_DEFAULT */
+   prefix = true;
+   case 'c':   /* KERN_CONT */
+   break;
+   }
+   text_len -= end_of_header - text;
+   text = (char *)end_of_header;
}
-   text_len -=  end_of_header - text;
-   text = (char *)end_of_header;
}
 
if (level == -1)


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs/ioctl.c: quiet sparse warnings

[Joe Perches]

On Fri, 2011-09-23 at 11:07 -0700, H Hartley Sweeten wrote:
 Quiet the following sparse warnings:
[]
 diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
[]
 @@ -2705,7 +2705,7 @@ long btrfs_ioctl_space_info(struct btrfs_root *root, 
 void __user *arg)
   up_read(info-groups_sem);
   }
  
 - user_dest = (struct btrfs_ioctl_space_info *)
 + user_dest = (struct btrfs_ioctl_space_info __user *)
 (arg + sizeof(struct btrfs_ioctl_space_args));

user_dest = arg;
user_dest++;

?


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 55/62] fs/btrfs/tree-log.c: Remove unnecessary semicolons

[Joe Perches]

Signed-off-by: Joe Perches j...@perches.com
---
 fs/btrfs/tree-log.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index c139222..ab326dd 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2605,7 +2605,7 @@ static noinline int copy_items(struct btrfs_trans_handle 
*trans,
extent);
cs = btrfs_file_extent_offset(src, extent);
cl = btrfs_file_extent_num_bytes(src,
-   extent);;
+   extent);
if (btrfs_file_extent_compression(src,
  extent)) {
cs = 0;
-- 
1.6.3.1.10.g659a0.dirty

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs: MAINTAINERS entry

[Joe Perches]

Now that btrfs is in mainline, perhaps
a maintainers entry is appropriate?

Perhaps:

diff --git a/MAINTAINERS b/MAINTAINERS
index 6f65a26..138a54c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1021,6 +1021,14 @@ M:   m...@bu3sch.de
 W: http://bu3sch.de/btgpio.php
 S: Maintained
 
+BTRFS FILE SYSTEM
+P: Chris Mason
+M: chris.ma...@oracle.com
+L: linux-btrfs@vger.kernel.org
+W: http://btrfs.wiki.kernel.org/
+T: git kernel.org:/pub/scm/linux/kernel/git/mason/btrfs-unstable.git
+S: Maintained
+
 BTTV VIDEO4LINUX DRIVER
 P: Mauro Carvalho Chehab
 M: mche...@infradead.org


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html