Re: [PATCH] mm:Make the function zap_huge_pmd bool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2015-07-02 12:03 PM, Theodore Ts'o wrote:
> On Thu, Jul 02, 2015 at 09:26:21AM +0200, Michal Hocko wrote:
>> On Wed 01-07-15 14:27:57, Nicholas Krause wrote:
>>> This makes the function zap_huge_pmd have a return type of bool
>>> now due to this particular function always returning one or zero
>>> as its return value.
>>
>> How does this help anything? IMO this just generates a pointless churn
>> in the code without a good reason.
> 
> Hi Michal,
> 
> My recommendation is to ignore patches sent by Nick.  In my experience
> he doesn't understand code before trying to make mechanical changes,
> and very few of his patches add any new value, and at least one that
> he tried to send me just 2 weeks or so ago (cherry-picked to try to
> "prove" why he had turned over a new leaf, so that I would support the
> removal of his e-mail address from being blacklisted on
> vger.kernel.org) was buggy, and when I asked him some basic questions
> about what the code was doing, it was clear he had no clue how the
> seq_file abstraction worked.  This didn't stop him from trying to
> patch the code, and if he had tested it, it would have crashed and
> burned instantly.
> 
> Of course, do whatevery you want, but IMHO it's not really not worth
> your time to deal with his patches, and if you reply, most people
> won't see his original e-mail since the vger.kernel.org blacklist is
> still in effect.
> 
> Regards,
> 
> 						- Ted
> 
Ted,
I looked into that patch further and would were correct it was wrong.
However here is a bug fix for the drm driver code that somebody else
stated was right but haven gotten a reply to from the maintainer and
have tried resending.
Nick
>From 2d2ddb5d9a2c4fcbae45339d4f775fcde49f36e1 Mon Sep 17 00:00:00 2001
From: Nicholas Krause <xerofoify@xxxxxxxxx>
Date: Wed, 13 May 2015 21:36:44 -0400
Subject: [PATCH 1/2] gma500:Add proper use of the variable ret for the
 function, psb_mmu_inset_pfn_sequence

This adds proper use of the variable ret by returning it
at the end of the function, psb_mmu_inset_pfn_sequence for
indicating to callers when an error has occurred. Further
more remove the unneeded double setting of ret to the error
code, -ENOMEM after checking if a call to the function,
psb_mmu_pt_alloc_map_lock is successful.

Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx>
---
 drivers/gpu/drm/gma500/mmu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
index 0eaf11c..d2c4bac 100644
--- a/drivers/gpu/drm/gma500/mmu.c
+++ b/drivers/gpu/drm/gma500/mmu.c
@@ -677,10 +677,9 @@ int psb_mmu_insert_pfn_sequence(struct psb_mmu_pd *pd, uint32_t start_pfn,
 	do {
 		next = psb_pd_addr_end(addr, end);
 		pt = psb_mmu_pt_alloc_map_lock(pd, addr);
-		if (!pt) {
-			ret = -ENOMEM;
+		if (!pt)
 			goto out;
-		}
+
 		do {
 			pte = psb_mmu_mask_pte(start_pfn++, type);
 			psb_mmu_set_pte(pt, addr, pte);
@@ -700,7 +699,7 @@ out:
 	if (pd->hw_context != -1)
 		psb_mmu_flush(pd->driver);
 
-	return 0;
+	return ret;
 }
 
 int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages,
-- 
2.1.4
>From f1802ff61ef69ff9ecaaeadb941d4ed725a0255a Mon Sep 17 00:00:00 2001
From: Nicholas Krause <xerofoify@xxxxxxxxx>
Date: Fri, 22 May 2015 23:10:49 -0400
Subject: [PATCH 2/2] gma500: Add new error checking for the function, psb_driver_load

This adds new error checking to the function, psb_driver_load
for when the function, psb_mmu_inset_pfn_sequence fails to grab
memory for the internal function call to psb_pd_addr_end. In
addition we now implement this by checking for a error code
return  value from the internal call to the function,
psb_mmu_inset_pfn_sequence for the function, psb_driver_load.
If a error code is returned we must jump to a new label,
unlock_err in order to deallocate our usage count by
one for the usage of the semaphore, sem before unloading
the drm_device structure and returning a error code.

Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx>
---
 drivers/gpu/drm/gma500/psb_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 92e7e57..4cef2c9 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -343,6 +343,8 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 					  dev_priv->stolen_base >> PAGE_SHIFT,
 					  pg->gatt_start,
 					  pg->stolen_size >> PAGE_SHIFT, 0);
+	if (ret)
+		goto unlock_err;
 	up_read(&pg->sem);
 
 	psb_mmu_set_pd_context(psb_mmu_get_default_pd(dev_priv->mmu), 0);
@@ -405,6 +407,8 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 #endif
 	/* Intel drm driver load is done, continue doing pvr load */
 	return 0;
+unlock_err:
+	up_read(&pg->sem);
 out_err:
 	psb_driver_unload(dev);
 	return ret;
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]