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

[Andrew Morton]

On Tue, 26 Feb 2013 16:55:57 +0800
Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx> wrote:

> ___ 2013___02___26___ 16:53, Andrew Morton ______:
> > On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/