[PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Chen Gang]

Like another p?d_alloc(), pud_alloc() also may fail, so need check it.

Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
---
 arch/metag/kernel/dma.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
index db589ad..e6cf39b 100644
--- a/arch/metag/kernel/dma.c
+++ b/arch/metag/kernel/dma.c
@@ -398,6 +398,11 @@ static int __init dma_alloc_init(void)
 		int offset = pgd_index(CONSISTENT_START);
 		pgd = pgd_offset(&init_mm, CONSISTENT_START);
 		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
+		if (!pud) {
+			pr_err("%s: no pud tables\n", __func__);
+			ret = -ENOMEM;
+			break;
+		}
 		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
 		if (!pmd) {
 			pr_err("%s: no pmd tables\n", __func__);
-- 
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[James Hogan]

On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
> 
> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>

NAK.

pud_alloc folds to pud_offset on Meta so it cannot fail.

Cheers
James

> ---
>  arch/metag/kernel/dma.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
> index db589ad..e6cf39b 100644
> --- a/arch/metag/kernel/dma.c
> +++ b/arch/metag/kernel/dma.c
> @@ -398,6 +398,11 @@ static int __init dma_alloc_init(void)
>  		int offset = pgd_index(CONSISTENT_START);
>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
> +		if (!pud) {
> +			pr_err("%s: no pud tables\n", __func__);
> +			ret = -ENOMEM;
> +			break;
> +		}
>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
>  		if (!pmd) {
>  			pr_err("%s: no pmd tables\n", __func__);
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Xishi Qiu]

On 2013/11/14 16:11, Chen Gang wrote:

> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
> 
> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/metag/kernel/dma.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
> index db589ad..e6cf39b 100644
> --- a/arch/metag/kernel/dma.c
> +++ b/arch/metag/kernel/dma.c
> @@ -398,6 +398,11 @@ static int __init dma_alloc_init(void)
>  		int offset = pgd_index(CONSISTENT_START);
>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
> +		if (!pud) {
> +			pr_err("%s: no pud tables\n", __func__);
> +			ret = -ENOMEM;
> +			break;
> +		}

It looks fine to me.

Thanks,
Xishi Qiu

>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
>  		if (!pmd) {
>  			pr_err("%s: no pmd tables\n", __func__);


--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Chen Gang]

On 11/14/2013 05:18 PM, James Hogan wrote:
> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>
>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> 
> NAK.
> 
> pud_alloc folds to pud_offset on Meta so it cannot fail.
> 

If so, can pmd_alloc() be fail?

I checked the related definitions, it seems a little complex, can we
assume p?d_alloc() are in the same condition?


Thanks.

> Cheers
> James
> 
>> ---
>>  arch/metag/kernel/dma.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
>> index db589ad..e6cf39b 100644
>> --- a/arch/metag/kernel/dma.c
>> +++ b/arch/metag/kernel/dma.c
>> @@ -398,6 +398,11 @@ static int __init dma_alloc_init(void)
>>  		int offset = pgd_index(CONSISTENT_START);
>>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
>> +		if (!pud) {
>> +			pr_err("%s: no pud tables\n", __func__);
>> +			ret = -ENOMEM;
>> +			break;
>> +		}
>>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
>>  		if (!pmd) {
>>  			pr_err("%s: no pmd tables\n", __func__);
> 
> 


-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Chen Gang]

On 11/14/2013 05:25 PM, Xishi Qiu wrote:
> On 2013/11/14 16:11, Chen Gang wrote:
> 
>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>
>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  arch/metag/kernel/dma.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
>> index db589ad..e6cf39b 100644
>> --- a/arch/metag/kernel/dma.c
>> +++ b/arch/metag/kernel/dma.c
>> @@ -398,6 +398,11 @@ static int __init dma_alloc_init(void)
>>  		int offset = pgd_index(CONSISTENT_START);
>>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
>> +		if (!pud) {
>> +			pr_err("%s: no pud tables\n", __func__);
>> +			ret = -ENOMEM;
>> +			break;
>> +		}
> 
> It looks fine to me.
> 

Thank you too.

James has his opinion about it, we are just discussing, welcome you
join.

:-)


Thanks.

> Thanks,
> Xishi Qiu
> 
>>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
>>  		if (!pmd) {
>>  			pr_err("%s: no pmd tables\n", __func__);
> 
> 
> 
> 
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[James Hogan]

On 14/11/13 09:35, Chen Gang wrote:
> On 11/14/2013 05:18 PM, James Hogan wrote:
>> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>>
>>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>
>> NAK.
>>
>> pud_alloc folds to pud_offset on Meta so it cannot fail.
>>
> 
> If so, can pmd_alloc() be fail?

No. pmd_alloc folds to pmd_offset so it also cannot fail, and the
existing !pmd check is dead code. The following would be better.

Cheers
James

From 85a386a9c7df666b1f438435be8a89581bc7e8b3 Mon Sep 17 00:00:00 2001
From: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Date: Thu, 14 Nov 2013 10:14:37 +0000
Subject: [PATCH 1/1] metag: dma: remove dead code in dma_alloc_init()

Meta has 2 levels of page table so the pmd folds into the pud which
folds into the pgd. Therefore the !pmd check in dma_alloc_init() is dead
code since it essentially checks whether:
  (init_mm->pgd + 0x770) == 0

Remove the check.

Reported-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 arch/metag/kernel/dma.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
index 8c00dedadc54..78205dbc2b86 100644
--- a/arch/metag/kernel/dma.c
+++ b/arch/metag/kernel/dma.c
@@ -401,11 +401,6 @@ static int __init dma_alloc_init(void)
 		pgd = pgd_offset(&init_mm, CONSISTENT_START);
 		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
 		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
-		if (!pmd) {
-			pr_err("%s: no pmd tables\n", __func__);
-			ret = -ENOMEM;
-			break;
-		}
 		WARN_ON(!pmd_none(*pmd));
 
 		pte = pte_alloc_kernel(pmd, CONSISTENT_START);
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Xishi Qiu]

On 2013/11/14 18:24, James Hogan wrote:

> On 14/11/13 09:35, Chen Gang wrote:
>> On 11/14/2013 05:18 PM, James Hogan wrote:
>>> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>>>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>>>
>>>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>>
>>> NAK.
>>>
>>> pud_alloc folds to pud_offset on Meta so it cannot fail.
>>>
>>
>> If so, can pmd_alloc() be fail?
> 
> No. pmd_alloc folds to pmd_offset so it also cannot fail, and the
> existing !pmd check is dead code. The following would be better.
> 
> Cheers
> James
> 
>>From 85a386a9c7df666b1f438435be8a89581bc7e8b3 Mon Sep 17 00:00:00 2001
> From: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Date: Thu, 14 Nov 2013 10:14:37 +0000
> Subject: [PATCH 1/1] metag: dma: remove dead code in dma_alloc_init()
> 
> Meta has 2 levels of page table so the pmd folds into the pud which

Hi James,

"Meta has 2 levels of page table", so it use
"#define pud_alloc(mm, pgd, address)	(pgd)", right?
and if the arch is x86_64, we should check the pmd and pud, right?

Thanks,
Xishi Qiu

> folds into the pgd. Therefore the !pmd check in dma_alloc_init() is dead
> code since it essentially checks whether:
>   (init_mm->pgd + 0x770) == 0
> 
> Remove the check.
> 
> Reported-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/metag/kernel/dma.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
> index 8c00dedadc54..78205dbc2b86 100644
> --- a/arch/metag/kernel/dma.c
> +++ b/arch/metag/kernel/dma.c
> @@ -401,11 +401,6 @@ static int __init dma_alloc_init(void)
>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
> -		if (!pmd) {
> -			pr_err("%s: no pmd tables\n", __func__);
> -			ret = -ENOMEM;
> -			break;
> -		}
>  		WARN_ON(!pmd_none(*pmd));
>  
>  		pte = pte_alloc_kernel(pmd, CONSISTENT_START);


--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Xishi Qiu]

On 2013/11/14 18:24, James Hogan wrote:

> On 14/11/13 09:35, Chen Gang wrote:
>> On 11/14/2013 05:18 PM, James Hogan wrote:
>>> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>>>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>>>
>>>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>>
>>> NAK.
>>>
>>> pud_alloc folds to pud_offset on Meta so it cannot fail.
>>>
>>
>> If so, can pmd_alloc() be fail?
> 
> No. pmd_alloc folds to pmd_offset so it also cannot fail, and the
> existing !pmd check is dead code. The following would be better.
> 
> Cheers
> James
> 

Hi James,

__PAGETABLE_PUD_FOLDED and __PAGETABLE_PMD_FOLDED are defined,
so __pud_alloc() and __pmd_alloc() always return 0, 
and pud_offset() just return (pud_t *)pgd, pmd_offset() just return (pmd_t *)pud,
is that right?

Thanks,
Xishi Qiu

>>From 85a386a9c7df666b1f438435be8a89581bc7e8b3 Mon Sep 17 00:00:00 2001
> From: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Date: Thu, 14 Nov 2013 10:14:37 +0000
> Subject: [PATCH 1/1] metag: dma: remove dead code in dma_alloc_init()
> 
> Meta has 2 levels of page table so the pmd folds into the pud which
> folds into the pgd. Therefore the !pmd check in dma_alloc_init() is dead
> code since it essentially checks whether:
>   (init_mm->pgd + 0x770) == 0
> 
> Remove the check.
> 
> Reported-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/metag/kernel/dma.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
> index 8c00dedadc54..78205dbc2b86 100644
> --- a/arch/metag/kernel/dma.c
> +++ b/arch/metag/kernel/dma.c
> @@ -401,11 +401,6 @@ static int __init dma_alloc_init(void)
>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
> -		if (!pmd) {
> -			pr_err("%s: no pmd tables\n", __func__);
> -			ret = -ENOMEM;
> -			break;
> -		}
>  		WARN_ON(!pmd_none(*pmd));
>  
>  		pte = pte_alloc_kernel(pmd, CONSISTENT_START);


--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[James Hogan]

On 14/11/13 11:33, Xishi Qiu wrote:
> On 2013/11/14 18:24, James Hogan wrote:
> 
>> On 14/11/13 09:35, Chen Gang wrote:
>>> On 11/14/2013 05:18 PM, James Hogan wrote:
>>>> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>>>>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>>>>
>>>>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>>>
>>>> NAK.
>>>>
>>>> pud_alloc folds to pud_offset on Meta so it cannot fail.
>>>>
>>>
>>> If so, can pmd_alloc() be fail?
>>
>> No. pmd_alloc folds to pmd_offset so it also cannot fail, and the
>> existing !pmd check is dead code. The following would be better.
>>
>> Cheers
>> James
>>
>> >From 85a386a9c7df666b1f438435be8a89581bc7e8b3 Mon Sep 17 00:00:00 2001
>> From: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> Date: Thu, 14 Nov 2013 10:14:37 +0000
>> Subject: [PATCH 1/1] metag: dma: remove dead code in dma_alloc_init()
>>
>> Meta has 2 levels of page table so the pmd folds into the pud which
> 
> Hi James,
> 
> "Meta has 2 levels of page table", so it use
> "#define pud_alloc(mm, pgd, address)	(pgd)", right?

Essentially yes. It uses the definitions in include/linux/mm.h:

> static inline pud_t *pud_alloc(struct mm_struct *mm, pgd_t *pgd, unsigned long address)
> {
> 	return (unlikely(pgd_none(*pgd)) && __pud_alloc(mm, pgd, address))?
> 		NULL: pud_offset(pgd, address);
> }
> 
> static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
> {
> 	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
> 		NULL: pmd_offset(pud, address);
> }

arch/metag/include/asm/pgtable.h includes asm-generic/pgtable-nopmd.h
which includes asm-generic/pgtable-nopud.h, so:

pgd_none() is always 0
__pud_alloc() is always 0
pud_alloc() = pud_offset() = pgd

pud_none() is always 0
__pmd_alloc() is always 0
pmd_alloc() = pmd_offset() = pud

> and if the arch is x86_64, we should check the pmd and pud, right?

I believe x86_64 uses 4 level page tables so yes, but you're probably
best off checking the relevant definitions, disassembling some code, or
asking the x86 maintainers. I believe there's been an article or two
about 4 level page tables on LWN too.

Cheers
James

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[James Hogan]

On 14/11/13 11:46, Xishi Qiu wrote:
> On 2013/11/14 18:24, James Hogan wrote:
> 
>> On 14/11/13 09:35, Chen Gang wrote:
>>> On 11/14/2013 05:18 PM, James Hogan wrote:
>>>> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>>>>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>>>>
>>>>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>>>
>>>> NAK.
>>>>
>>>> pud_alloc folds to pud_offset on Meta so it cannot fail.
>>>>
>>>
>>> If so, can pmd_alloc() be fail?
>>
>> No. pmd_alloc folds to pmd_offset so it also cannot fail, and the
>> existing !pmd check is dead code. The following would be better.
>>
>> Cheers
>> James
>>
> 
> Hi James,
> 
> __PAGETABLE_PUD_FOLDED and __PAGETABLE_PMD_FOLDED are defined,
> so __pud_alloc() and __pmd_alloc() always return 0, 
> and pud_offset() just return (pud_t *)pgd, pmd_offset() just return (pmd_t *)pud,
> is that right?

Yes exactly (although technically __pud_alloc() and __pmd_alloc() would
never get called due to pgd_none() and pud_none() returning 0, but it
makes little difference).

Cheers
James

> 
>> >From 85a386a9c7df666b1f438435be8a89581bc7e8b3 Mon Sep 17 00:00:00 2001
>> From: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> Date: Thu, 14 Nov 2013 10:14:37 +0000
>> Subject: [PATCH 1/1] metag: dma: remove dead code in dma_alloc_init()
>>
>> Meta has 2 levels of page table so the pmd folds into the pud which
>> folds into the pgd. Therefore the !pmd check in dma_alloc_init() is dead
>> code since it essentially checks whether:
>>   (init_mm->pgd + 0x770) == 0
>>
>> Remove the check.
>>
>> Reported-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> ---
>>  arch/metag/kernel/dma.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
>> index 8c00dedadc54..78205dbc2b86 100644
>> --- a/arch/metag/kernel/dma.c
>> +++ b/arch/metag/kernel/dma.c
>> @@ -401,11 +401,6 @@ static int __init dma_alloc_init(void)
>>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
>>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
>> -		if (!pmd) {
>> -			pr_err("%s: no pmd tables\n", __func__);
>> -			ret = -ENOMEM;
>> -			break;
>> -		}
>>  		WARN_ON(!pmd_none(*pmd));
>>  
>>  		pte = pte_alloc_kernel(pmd, CONSISTENT_START);
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arch: metag: kernel: dma.c: check 'pud' whether is NULL in dma_alloc_init()

[Chen Gang]

On 11/14/2013 07:51 PM, James Hogan wrote:
> On 14/11/13 11:46, Xishi Qiu wrote:
>> On 2013/11/14 18:24, James Hogan wrote:
>>
>>> On 14/11/13 09:35, Chen Gang wrote:
>>>> On 11/14/2013 05:18 PM, James Hogan wrote:
>>>>> On Thursday 14 November 2013 16:11:21 Chen Gang wrote:
>>>>>> Like another p?d_alloc(), pud_alloc() also may fail, so need check it.
>>>>>>
>>>>>> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>>>>
>>>>> NAK.
>>>>>
>>>>> pud_alloc folds to pud_offset on Meta so it cannot fail.
>>>>>
>>>>
>>>> If so, can pmd_alloc() be fail?
>>>
>>> No. pmd_alloc folds to pmd_offset so it also cannot fail, and the
>>> existing !pmd check is dead code. The following would be better.
>>>
>>> Cheers
>>> James
>>>
>>
>> Hi James,
>>
>> __PAGETABLE_PUD_FOLDED and __PAGETABLE_PMD_FOLDED are defined,
>> so __pud_alloc() and __pmd_alloc() always return 0, 
>> and pud_offset() just return (pud_t *)pgd, pmd_offset() just return (pmd_t *)pud,
>> is that right?
> 
> Yes exactly (although technically __pud_alloc() and __pmd_alloc() would
> never get called due to pgd_none() and pud_none() returning 0, but it
> makes little difference).
> 

Thank all of you (originally, I did not notice about "linux/mm.h"
include "asm/pgtable.h" -- which maybe be a common sense).

:-)

> Cheers
> James
> 
>>
>>> >From 85a386a9c7df666b1f438435be8a89581bc7e8b3 Mon Sep 17 00:00:00 2001
>>> From: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>> Date: Thu, 14 Nov 2013 10:14:37 +0000
>>> Subject: [PATCH 1/1] metag: dma: remove dead code in dma_alloc_init()
>>>
>>> Meta has 2 levels of page table so the pmd folds into the pud which
>>> folds into the pgd. Therefore the !pmd check in dma_alloc_init() is dead
>>> code since it essentially checks whether:
>>>   (init_mm->pgd + 0x770) == 0
>>>
>>> Remove the check.
>>>
>>> Reported-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>>> Signed-off-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  arch/metag/kernel/dma.c | 5 -----
>>>  1 file changed, 5 deletions(-)
>>>
>>> diff --git a/arch/metag/kernel/dma.c b/arch/metag/kernel/dma.c
>>> index 8c00dedadc54..78205dbc2b86 100644
>>> --- a/arch/metag/kernel/dma.c
>>> +++ b/arch/metag/kernel/dma.c
>>> @@ -401,11 +401,6 @@ static int __init dma_alloc_init(void)
>>>  		pgd = pgd_offset(&init_mm, CONSISTENT_START);
>>>  		pud = pud_alloc(&init_mm, pgd, CONSISTENT_START);
>>>  		pmd = pmd_alloc(&init_mm, pud, CONSISTENT_START);
>>> -		if (!pmd) {
>>> -			pr_err("%s: no pmd tables\n", __func__);
>>> -			ret = -ENOMEM;
>>> -			break;
>>> -		}
>>>  		WARN_ON(!pmd_none(*pmd));
>>>  
>>>  		pte = pte_alloc_kernel(pmd, CONSISTENT_START);
>>
>>
> 
> 
> 


-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html