On 2015-07-03 10:46 AM, Theodore Ts'o wrote:
> On Thu, Jul 02, 2015 at 12:08:36PM -0400, nick wrote:
>> 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.
>
> Hi Nick,
>
> Don't bother sending more low-value patches like this; they don't
> impress me. Send me a patch that fixes a deep bug, where you can
> demonstrate that you understand the underlying design of the code, can
> point out a flaw, and then explain why your patch is an improvement,
> and documents how you tested it. Or do something beyond changing
> return values or return types, and optimize some performance-critical
> part of the kernel, and in the commit description, explain why it
> improves things, how you measured the performance improvement, and why
> this is applicable in a real-life situation.
>
> Even a broken clock can be right twice a day, and the fact that it is
> possible that you can author a correct patch isn't all that
> impressive. You need to understand deep understanding of the code you
> are modifying, and or else it's not worth my time to go through a
> large number of low-value patches that don't really improve the code
> base much, when the risk that you have accidentally introduced a bug
> is high given that (a) you've demonstrated an inability to explain
> some of your patches, and (b) in many cases, you have no fear about
> sending patches that you can't personally test. These two
> shortcomings in combination are fatal.
>
> If you can demonstrate that you can become a thoughtful and careful
> coder, I would be most pleased to argue to Greg K-H that you have
> turned over a new leaf. To date, however, you have not demonstrated
> any of the above, and you've made me regret that I've tried to waste
> time looking at your patches that you've sent me in the hopes of
> convincing me that you've really changed --- when it's clear you
> haven't. I do hope that, one day, you will be able to be a good
> coder. But that day is clearly not today.
>
> Best regards,
>
> - Ted
>
Did you even look at the other patches I send you. Here is a bug fix for the gma500 driver code
that someone else stated is right but I don't have the hardware so it's difficult to test.
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.4From 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>