Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-27 Thread Andrew Morton 
On Tue, 26 Feb 2013 16:55:57 +0800
Zhang Yanfei  wrote:

> ___ 2013___02___26___ 16:53, Andrew Morton __:
> > On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei 
> >  wrote:
> > 
>  diff --git a/kernel/kexec.c b/kernel/kexec.c
> >>> []
>  @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
>  *image,
> >>> []
>  +mchunk = min_t(size_t, mbytes,
>  +   (size_t)(PAGE_SIZE - (maddr & 
>  ~PAGE_MASK)));
> >>>
> >>> #define min_t(type, x, y) ({  \
> >>>   type __min1 = (x);  \
> >>>   type __min2 = (y);  \
> >>>   __min1 < __min2 ? __min1: __min2; })
> >>>
> >>>
> >>>
> >>
> >> Hmm, from the definition, the cast is redundant.
> >>
> >> Maybe I misunderstood what Andrew meant in the mail related to v2:
> >>
> >> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> >> different types on different architectures, so some form of casting is
> >> unavoidable here."
> >>
> >> Andrew, could you please explain the casting you meant above?
> > 
> > I mean that a cast (or min_t, which is a cast) will be needed. 
> > The code you have here casts the same thing two times, which isn't
> > necessary.
> > 
> > 
> 
> Thanks.
> 
> Should I resend the patch again? After removing this redundant cast,
> it is the same with v2.

That patch was pretty busted.   This should fix everything up:

--- a/kernel/kexec.c~kexec-use-min_t-to-simplify-logic-fix
+++ a/kernel/kexec.c
@@ -820,8 +820,8 @@ static int kimage_load_normal_segment(st
clear_page(ptr);
ptr += maddr & ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
-  (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
-   uchunk = min_t(size_t, ubytes, mchunk);
+   PAGE_SIZE - (maddr & ~PAGE_MASK));
+   uchunk = min(ubytes, mchunk);
 
result = copy_from_user(ptr, buf, uchunk);
kunmap(page);
@@ -868,8 +868,8 @@ static int kimage_load_crash_segment(str
ptr = kmap(page);
ptr += maddr & ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
-  (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
-   uchunk = min_t(size_t, ubytes, mchunk);
+   PAGE_SIZE - (maddr & ~PAGE_MASK));
+   uchunk = min(ubytes, mchunk);
if (mchunk > uchunk) {
/* Zero the trailing part of the page */
memset(ptr + uchunk, 0, mchunk - uchunk);
@@ -1451,7 +1451,7 @@ void vmcoreinfo_append_str(const char *f
r = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
 
-   r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
+   r = min(r, vmcoreinfo_max_size - vmcoreinfo_size);
 
memcpy(_data[vmcoreinfo_size], buf, r);
 
_

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

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-27 Thread Andrew Morton 
On Tue, 26 Feb 2013 16:55:57 +0800
Zhang Yanfei zhangyan...@cn.fujitsu.com wrote:

 ___ 2013___02___26___ 16:53, Andrew Morton __:
  On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei 
  zhangyan...@cn.fujitsu.com wrote:
  
  diff --git a/kernel/kexec.c b/kernel/kexec.c
  []
  @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
  *image,
  []
  +mchunk = min_t(size_t, mbytes,
  +   (size_t)(PAGE_SIZE - (maddr  
  ~PAGE_MASK)));
 
  #define min_t(type, x, y) ({  \
type __min1 = (x);  \
type __min2 = (y);  \
__min1  __min2 ? __min1: __min2; })
 
 
 
 
  Hmm, from the definition, the cast is redundant.
 
  Maybe I misunderstood what Andrew meant in the mail related to v2:
 
  The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
  different types on different architectures, so some form of casting is
  unavoidable here.
 
  Andrew, could you please explain the casting you meant above?
  
  I mean that a cast (or min_t, which is a cast) will be needed. 
  The code you have here casts the same thing two times, which isn't
  necessary.
  
  
 
 Thanks.
 
 Should I resend the patch again? After removing this redundant cast,
 it is the same with v2.

That patch was pretty busted.   This should fix everything up:

--- a/kernel/kexec.c~kexec-use-min_t-to-simplify-logic-fix
+++ a/kernel/kexec.c
@@ -820,8 +820,8 @@ static int kimage_load_normal_segment(st
clear_page(ptr);
ptr += maddr  ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
-  (size_t)(PAGE_SIZE - (maddr  ~PAGE_MASK)));
-   uchunk = min_t(size_t, ubytes, mchunk);
+   PAGE_SIZE - (maddr  ~PAGE_MASK));
+   uchunk = min(ubytes, mchunk);
 
result = copy_from_user(ptr, buf, uchunk);
kunmap(page);
@@ -868,8 +868,8 @@ static int kimage_load_crash_segment(str
ptr = kmap(page);
ptr += maddr  ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
-  (size_t)(PAGE_SIZE - (maddr  ~PAGE_MASK)));
-   uchunk = min_t(size_t, ubytes, mchunk);
+   PAGE_SIZE - (maddr  ~PAGE_MASK));
+   uchunk = min(ubytes, mchunk);
if (mchunk  uchunk) {
/* Zero the trailing part of the page */
memset(ptr + uchunk, 0, mchunk - uchunk);
@@ -1451,7 +1451,7 @@ void vmcoreinfo_append_str(const char *f
r = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
 
-   r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
+   r = min(r, vmcoreinfo_max_size - vmcoreinfo_size);
 
memcpy(vmcoreinfo_data[vmcoreinfo_size], buf, r);
 
_

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

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Zhang Yanfei 
于 2013年02月26日 16:53, Andrew Morton 写道:
> On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei  
> wrote:
> 
 diff --git a/kernel/kexec.c b/kernel/kexec.c
>>> []
 @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
 *image,
>>> []
 +  mchunk = min_t(size_t, mbytes,
 + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
>>>
>>> #define min_t(type, x, y) ({\
>>> type __min1 = (x);  \
>>> type __min2 = (y);  \
>>> __min1 < __min2 ? __min1: __min2; })
>>>
>>>
>>>
>>
>> Hmm, from the definition, the cast is redundant.
>>
>> Maybe I misunderstood what Andrew meant in the mail related to v2:
>>
>> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
>> different types on different architectures, so some form of casting is
>> unavoidable here."
>>
>> Andrew, could you please explain the casting you meant above?
> 
> I mean that a cast (or min_t, which is a cast) will be needed. 
> The code you have here casts the same thing two times, which isn't
> necessary.
> 
> 

Thanks.

Should I resend the patch again? After removing this redundant cast,
it is the same with v2.

Zhang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Andrew Morton 
On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei  
wrote:

> >> diff --git a/kernel/kexec.c b/kernel/kexec.c
> > []
> >> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
> >> *image,
> > []
> >> +  mchunk = min_t(size_t, mbytes,
> >> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> > 
> > #define min_t(type, x, y) ({\
> > type __min1 = (x);  \
> > type __min2 = (y);  \
> > __min1 < __min2 ? __min1: __min2; })
> > 
> > 
> > 
> 
> Hmm, from the definition, the cast is redundant.
> 
> Maybe I misunderstood what Andrew meant in the mail related to v2:
> 
> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> different types on different architectures, so some form of casting is
> unavoidable here."
> 
> Andrew, could you please explain the casting you meant above?

I mean that a cast (or min_t, which is a cast) will be needed. 
The code you have here casts the same thing two times, which isn't
necessary.

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

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Zhang Yanfei 
于 2013年02月26日 16:38, Joe Perches 写道:
> On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
>> This is just a tweak: using min_t to simplify logic of variable
>> assignments.
>>
>> v3:
>> - cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
> 
> Why?  Isn't this just a redundant cast?
> 
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
> []
>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
>> *image,
> []
>> +mchunk = min_t(size_t, mbytes,
>> +   (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> 
> #define min_t(type, x, y) ({  \
>   type __min1 = (x);  \
>   type __min2 = (y);  \
>   __min1 < __min2 ? __min1: __min2; })
> 
> 
> 

Hmm, from the definition, the cast is redundant.

Maybe I misunderstood what Andrew meant in the mail related to v2:

"The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
different types on different architectures, so some form of casting is
unavoidable here."

Andrew, could you please explain the casting you meant above?

Thanks
Zhang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Joe Perches 
On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
> This is just a tweak: using min_t to simplify logic of variable
> assignments.
> 
> v3:
> - cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.

Why?  Isn't this just a redundant cast?

> diff --git a/kernel/kexec.c b/kernel/kexec.c
[]
> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
[]
> + mchunk = min_t(size_t, mbytes,
> +(size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));

#define min_t(type, x, y) ({\
type __min1 = (x);  \
type __min2 = (y);  \
__min1 < __min2 ? __min1: __min2; })


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

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Joe Perches 
On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
 This is just a tweak: using min_t to simplify logic of variable
 assignments.
 
 v3:
 - cast type of (PAGE_SIZE - (maddr  ~PAGE_MASK)) into size_t.

Why?  Isn't this just a redundant cast?

 diff --git a/kernel/kexec.c b/kernel/kexec.c
[]
 @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
 *image,
[]
 + mchunk = min_t(size_t, mbytes,
 +(size_t)(PAGE_SIZE - (maddr  ~PAGE_MASK)));

#define min_t(type, x, y) ({\
type __min1 = (x);  \
type __min2 = (y);  \
__min1  __min2 ? __min1: __min2; })


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

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Zhang Yanfei 
于 2013年02月26日 16:38, Joe Perches 写道:
 On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
 This is just a tweak: using min_t to simplify logic of variable
 assignments.

 v3:
 - cast type of (PAGE_SIZE - (maddr  ~PAGE_MASK)) into size_t.
 
 Why?  Isn't this just a redundant cast?
 
 diff --git a/kernel/kexec.c b/kernel/kexec.c
 []
 @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
 *image,
 []
 +mchunk = min_t(size_t, mbytes,
 +   (size_t)(PAGE_SIZE - (maddr  ~PAGE_MASK)));
 
 #define min_t(type, x, y) ({  \
   type __min1 = (x);  \
   type __min2 = (y);  \
   __min1  __min2 ? __min1: __min2; })
 
 
 

Hmm, from the definition, the cast is redundant.

Maybe I misunderstood what Andrew meant in the mail related to v2:

The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
different types on different architectures, so some form of casting is
unavoidable here.

Andrew, could you please explain the casting you meant above?

Thanks
Zhang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Andrew Morton 
On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei zhangyan...@cn.fujitsu.com 
wrote:

  diff --git a/kernel/kexec.c b/kernel/kexec.c
  []
  @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
  *image,
  []
  +  mchunk = min_t(size_t, mbytes,
  + (size_t)(PAGE_SIZE - (maddr  ~PAGE_MASK)));
  
  #define min_t(type, x, y) ({\
  type __min1 = (x);  \
  type __min2 = (y);  \
  __min1  __min2 ? __min1: __min2; })
  
  
  
 
 Hmm, from the definition, the cast is redundant.
 
 Maybe I misunderstood what Andrew meant in the mail related to v2:
 
 The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
 different types on different architectures, so some form of casting is
 unavoidable here.
 
 Andrew, could you please explain the casting you meant above?

I mean that a cast (or min_t, which is a cast) will be needed. 
The code you have here casts the same thing two times, which isn't
necessary.

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

Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

2013-02-26 Thread Zhang Yanfei 
于 2013年02月26日 16:53, Andrew Morton 写道:
 On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei zhangyan...@cn.fujitsu.com 
 wrote:
 
 diff --git a/kernel/kexec.c b/kernel/kexec.c
 []
 @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage 
 *image,
 []
 +  mchunk = min_t(size_t, mbytes,
 + (size_t)(PAGE_SIZE - (maddr  ~PAGE_MASK)));

 #define min_t(type, x, y) ({\
 type __min1 = (x);  \
 type __min2 = (y);  \
 __min1  __min2 ? __min1: __min2; })




 Hmm, from the definition, the cast is redundant.

 Maybe I misunderstood what Andrew meant in the mail related to v2:

 The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
 different types on different architectures, so some form of casting is
 unavoidable here.

 Andrew, could you please explain the casting you meant above?
 
 I mean that a cast (or min_t, which is a cast) will be needed. 
 The code you have here casts the same thing two times, which isn't
 necessary.
 
 

Thanks.

Should I resend the patch again? After removing this redundant cast,
it is the same with v2.

Zhang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/