Re: [GIT PULL] x86/mm fixes

Re: [GIT PULL] x86/mm fixes

[Linus Torvalds]

On Tue, 2 Mar 2010, H. Peter Anvin wrote:
> +		pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
> +		end_pfn = (res.end + 1) >> PAGE_SHIFT;
> +		if (end_pfn > pfn)
> +		    ret = (*func)(pfn, end_pfn - pfn, arg);
>  		if (ret)
>  			break;
>  		res.start = res.end + 1;

What kind of messed-up indentation is that? We don't use 4-char indents.

		Linus

Re: [GIT PULL] x86/mm fixes

[H. Peter Anvin]

On 03/02/2010 10:39 AM, Linus Torvalds wrote:
> 
> 
> On Tue, 2 Mar 2010, H. Peter Anvin wrote:
>> +		pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> +		end_pfn = (res.end + 1) >> PAGE_SHIFT;
>> +		if (end_pfn > pfn)
>> +		    ret = (*func)(pfn, end_pfn - pfn, arg);
>>  		if (ret)
>>  			break;
>>  		res.start = res.end + 1;
> 
> What kind of messed-up indentation is that? We don't use 4-char indents.
> 
> 		Linus

Sorry, you're right.  I have to admit I didn't even see that aspect of
the patch.

I'll add a reformatting patch and update the branch, if you want.

	-hpa

Re: [GIT PULL] x86/mm fixes

[H. Peter Anvin]

On 03/02/2010 10:39 AM, Linus Torvalds wrote:
> 
> 
> On Tue, 2 Mar 2010, H. Peter Anvin wrote:
>> +		pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> +		end_pfn = (res.end + 1) >> PAGE_SHIFT;
>> +		if (end_pfn > pfn)
>> +		    ret = (*func)(pfn, end_pfn - pfn, arg);
>>  		if (ret)
>>  			break;
>>  		res.start = res.end + 1;
> 
> What kind of messed-up indentation is that? We don't use 4-char indents.
> 

Branch updated with an indentation patch.  Sorry about that.

	-hpa

Re: [GIT PULL] x86/mm fixes

[Wu Fengguang]

On Wed, Mar 03, 2010 at 03:31:48AM +0800, H. Peter Anvin wrote:
> On 03/02/2010 10:39 AM, Linus Torvalds wrote:
> > 
> > 
> > On Tue, 2 Mar 2010, H. Peter Anvin wrote:
> >> +		pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
> >> +		end_pfn = (res.end + 1) >> PAGE_SHIFT;
> >> +		if (end_pfn > pfn)
> >> +		    ret = (*func)(pfn, end_pfn - pfn, arg);
> >>  		if (ret)
> >>  			break;
> >>  		res.start = res.end + 1;
> > 
> > What kind of messed-up indentation is that? We don't use 4-char indents.
> > 
> 
> Branch updated with an indentation patch.  Sorry about that.

Sorry! Wondering I didn't do the indent with the vim '=='.

Anyway I just hacked /usr/share/quilt/refresh to automatically run the
kernel style checker:

# wfg: check for kernel coding style
if [ -x scripts/checkpatch.pl ]; then
        scripts/checkpatch.pl $patch_file
fi

Thanks,
Fengguang

Re: [GIT PULL] x86/mm fixes

[Linus Torvalds]

On Wed, 3 Mar 2010, Wu Fengguang wrote:
> 
> Anyway I just hacked /usr/share/quilt/refresh to automatically run the
> kernel style checker:
> 
> # wfg: check for kernel coding style
> if [ -x scripts/checkpatch.pl ]; then
>         scripts/checkpatch.pl $patch_file
> fi

It's probably sensible to check things, but do remember that some of the 
things checkpatch warns about are better left the way they are, rather 
than make the code uglier just to make checkpatch happy.

			Linus

Re: [GIT PULL] x86/mm fixes

[Wu Fengguang]

On Thu, Mar 04, 2010 at 01:10:46AM +0800, Linus Torvalds wrote:
> 
> 
> On Wed, 3 Mar 2010, Wu Fengguang wrote:
> > 
> > Anyway I just hacked /usr/share/quilt/refresh to automatically run the
> > kernel style checker:
> > 
> > # wfg: check for kernel coding style
> > if [ -x scripts/checkpatch.pl ]; then
> >         scripts/checkpatch.pl $patch_file
> > fi
> 
> It's probably sensible to check things, but do remember that some of the 
> things checkpatch warns about are better left the way they are, rather 
> than make the code uglier just to make checkpatch happy.

OK. Here is the simple patch for quilt. I guess it may be hardly
acceptable for quilt upstream, so only intends to share it here.

Thanks,
Fengguang
---

--- /usr/share/quilt/refresh.orig	2010-03-04 08:38:27.000000000 +0800
+++ /usr/share/quilt/refresh	2010-03-04 08:38:29.000000000 +0800
@@ -265,6 +265,12 @@ fi
 
 patch_file=$(patch_file_name $patch)
 
+# check for kernel coding style
+# Linus: please treat its warning as recommendation instead of requirement
+if [ -x scripts/checkpatch.pl ]; then
+	scripts/checkpatch.pl -q $patch_file
+fi
+
 trap "" SIGINT
 
 tmp_result=$(gen_tempfile) || die 1

Re: [GIT PULL] x86/mm fixes

[Peter Zijlstra]

On Thu, 2010-03-04 at 08:44 +0800, Wu Fengguang wrote:
> On Thu, Mar 04, 2010 at 01:10:46AM +0800, Linus Torvalds wrote:
> > 
> > 
> > On Wed, 3 Mar 2010, Wu Fengguang wrote:
> > > 
> > > Anyway I just hacked /usr/share/quilt/refresh to automatically run the
> > > kernel style checker:
> > > 
> > > # wfg: check for kernel coding style
> > > if [ -x scripts/checkpatch.pl ]; then
> > >         scripts/checkpatch.pl $patch_file
> > > fi
> > 
> > It's probably sensible to check things, but do remember that some of the 
> > things checkpatch warns about are better left the way they are, rather 
> > than make the code uglier just to make checkpatch happy.
> 
> OK. Here is the simple patch for quilt. I guess it may be hardly
> acceptable for quilt upstream, so only intends to share it here.

I use the below, which allows me to specify what to run on refresh time.

The script I have it use looks like:

$ cat /usr/local/bin/fix-patch.sh
TMP=`tempfile -d /tmp`
awk -f /usr/local/bin/fix-patch.awk $1 > $TMP
if [ -x ./scripts/checkpatch.pl ]; then
        ./scripts/checkpatch.pl $TMP
fi
mv $TMP $1


and fix-patch.awk does things like sanitize mail headers and add
Signed-off-by, lines.

---

--- /usr/share/quilt/refresh~	2008-06-22 00:43:27.000000000 +0200
+++ /usr/share/quilt/refresh	2008-05-28 12:45:27.789068034 +0200
@@ -278,6 +278,10 @@
 
 cat $tmp_patch >> $tmp_result
 
+if [ -n "$QUILT_REFRESH_EXT" ] ; then
+	$QUILT_REFRESH_EXT $tmp_result
+fi
+
 if [ -e $patch_file ] && \
    diff -q $patch_file $tmp_result > /dev/null
 then

Re: [GIT PULL] x86/mm fixes

[Wu Fengguang]

On Thu, Mar 04, 2010 at 11:03:35PM +0800, Peter Zijlstra wrote:
> On Thu, 2010-03-04 at 08:44 +0800, Wu Fengguang wrote:
> > On Thu, Mar 04, 2010 at 01:10:46AM +0800, Linus Torvalds wrote:
> > > 
> > > 
> > > On Wed, 3 Mar 2010, Wu Fengguang wrote:
> > > > 
> > > > Anyway I just hacked /usr/share/quilt/refresh to automatically run the
> > > > kernel style checker:
> > > > 
> > > > # wfg: check for kernel coding style
> > > > if [ -x scripts/checkpatch.pl ]; then
> > > >         scripts/checkpatch.pl $patch_file
> > > > fi
> > > 
> > > It's probably sensible to check things, but do remember that some of the 
> > > things checkpatch warns about are better left the way they are, rather 
> > > than make the code uglier just to make checkpatch happy.
> > 
> > OK. Here is the simple patch for quilt. I guess it may be hardly
> > acceptable for quilt upstream, so only intends to share it here.
> 
> I use the below, which allows me to specify what to run on refresh time.
> 
> The script I have it use looks like:
> 
> $ cat /usr/local/bin/fix-patch.sh
> TMP=`tempfile -d /tmp`
> awk -f /usr/local/bin/fix-patch.awk $1 > $TMP
> if [ -x ./scripts/checkpatch.pl ]; then
>         ./scripts/checkpatch.pl $TMP

FYI, I added "-q" to checkpatch.pl to shut it up when patch is OK.

> fi
> mv $TMP $1
> 
> 
> and fix-patch.awk does things like sanitize mail headers and add
> Signed-off-by, lines.

Nice tool. Can you share fix-patch.awk? Thanks!

Regards,
Fengguang

> ---
> 
> --- /usr/share/quilt/refresh~	2008-06-22 00:43:27.000000000 +0200
> +++ /usr/share/quilt/refresh	2008-05-28 12:45:27.789068034 +0200
> @@ -278,6 +278,10 @@
>  
>  cat $tmp_patch >> $tmp_result
>  
> +if [ -n "$QUILT_REFRESH_EXT" ] ; then
> +	$QUILT_REFRESH_EXT $tmp_result
> +fi
> +
>  if [ -e $patch_file ] && \
>     diff -q $patch_file $tmp_result > /dev/null
>  then
> 
>