A question about toot1/vm/page-type.c
Hi, guys My system have 4G memory. I print the following memssage. I execute the command (page-types) to obtain all pfn status. I check 784272 pfn about 3063M from page-types , but I ought to obtain all pfn status in the whole memory space, is right ? The Memtotal is ~2932M. therefore, I guess that page-types do not show the reserved pages from the following memssage. [root@localhost vm]# ./page-types flags page-count MB symbolic-flags long-symbolic-flags 0x 89366 349 __ 0x0100 10 z_ zero_page 0x0008 130 ___U__ uptodate 0x000c 10 __RU__ referenced,uptodate 0x0028 44175 172 ___U_l uptodate,lru 0x002c 39128 152 __RU_l referenced,uptodate,lru 0x4030 8763 Dlb___ dirty,lru,swapbacked 0x4038 210 ___UDlb___ uptodate,dirty,lru,swapbacked 0x403c 1240 __RUDlb___ referenced,uptodate,dirty,lru,swapbacked 0x0068 303508 1185 ___U_lA___ uptodate,lru,active 0x006c 146546 572 __RU_lA___ referenced,uptodate,lru,active 0x4078 170 ___UDlA___b___ uptodate,dirty,lru,active,swapbacked 0x407c 40 __RUDlA___b___ referenced,uptodate,dirty,lru,active,swapbacked 0x0080 97203 379 ___S__ slab 0x0228 4331 ___U_l___I uptodate,lru,reclaim 0x0268 5992 ___U_lA__I uptodate,lru,active,reclaim 0x0400 36344 141 __B___ buddy 0x0800 6732 ___M__ mmap 0x0804 10 __RM__ referenced,mmap 0x0828 670 ___U_l_M__ uptodate,lru,mmap 0x082c 4351 __RU_l_M__ referenced,uptodate,lru,mmap 0x4838 7112 ___UDl_M__b___ uptodate,dirty,lru,mmap,swapbacked 0x483c 10 __RUDl_M__b___ referenced,uptodate,dirty,lru,mmap,swapbacked 0x0868 2240 ___U_lAM__ uptodate,lru,active,mmap 0x086c5062 19 __RU_lAM__ referenced,uptodate,lru,active,mmap 0x4878 4631 ___UDlAM__b___ uptodate,dirty,lru,active,mmap,swapbacked 0x5048 380 ___U__A_a_b___ uptodate,active,anonymous,swapbacked 0x004018003577 13 ___Ma_t___ mmap,anonymous,thp 0x5828 10863 42 ___U_l_Ma_b___ uptodate,lru,mmap,anonymous,swapbacked 0x00405828 20 ___U_l_Ma_b___t___ uptodate,lru,mmap,anonymous,swapbacked,thp 0x582c 350 __RU_l_Ma_b___ referenced,uptodate,lru,mmap,anonymous,swapbacked 0x5838 40 ___UDl_Ma_b___ uptodate,dirty,lru,mmap,anonymous,swapbacked 0x0040583c 10 __RUDl_Ma_b___t___ referenced,uptodate,dirty,lru,mmap,anonymous,swapbacked,thp 0x5848 200 ___U__AMa_b___ uptodate,active,mmap,anonymous,swapbacked 0x58683716 14 ___U_lAMa_b___ uptodate,lru,active,mmap,anonymous,swapbacked 0x00405868 40 ___U_lAMa_b___t___ uptodate,lru,active,mmap,anonymous,swapbacked,thp 0x586c 160 __RU_lAMa_b___ referenced,uptodate,lru,active,mmap,anonymous,swapbacked total 784272 3063 [root@localhost vm]# cat
Re: [PATCH] mm/huge_memory: fix the memory leak due to the race
On 2016/6/21 23:29, Kirill A. Shutemov wrote: > On Tue, Jun 21, 2016 at 11:19:07PM +0800, zhong jiang wrote: >> On 2016/6/21 22:37, Kirill A. Shutemov wrote: >>> On Tue, Jun 21, 2016 at 10:05:56PM +0800, zhongjiang wrote: >>>> From: zhong jiang >>>> >>>> with great pressure, I run some test cases. As a result, I found >>>> that the THP is not freed, it is detected by check_mm(). >>>> >>>> BUG: Bad rss-counter state mm:8827edb7 idx:1 val:512 >>>> >>>> Consider the following race : >>>> >>>>CPU0 CPU1 >>>> __handle_mm_fault() >>>> wp_huge_pmd() >>>>do_huge_pmd_wp_page() >>>>pmdp_huge_clear_flush_notify() >>>> (pmd_none = true) >>>>exit_mmap() >>>> unmap_vmas() >>>> zap_pmd_range() >>>> >>>> pmd_none_or_trans_huge_or_clear_bad() >>>> (result in memory leak) >>>> set_pmd_at() >>>> >>>> because of CPU0 have allocated huge page before pmdp_huge_clear_notify, >>>> and it make the pmd entry to be null. Therefore, The memory leak can occur. >>>> >>>> The patch fix the scenario that the pmd entry can lead to be null. >>> I don't think the scenario is possible. >>> >>> exit_mmap() called when all mm users have gone, so no parallel threads >>> exist. >>> >> Forget this patch. It 's my fault , it indeed don not exist. >> But I hit the following problem. we can see the memory leak when the >> process exit. >> >> >> Any suggestion will be apprecaited. > Could you try this: > > http://lkml.kernel.org/r/20160621150433.ga7...@node.shutemov.name > I fails to open it. can you display or add attachmemts ? :-) thx
[RFC] remove unnecessary condition in remove_inode_hugepages
At present, we need to call hugetlb_fix_reserve_count when
hugetlb_unrserve_pages fails,
and PagePrivate will decide hugetlb reserves counts.
we obtain the page from page cache. and use page both lock_page and mutex_lock.
alloc_huge_page add page to page chace always hold lock page, then bail out
clearpageprivate
before unlock page.
but I' m not sure it is right or I miss the points.
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 4ea71eb..010723b 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -462,14 +462,12 @@ static void remove_inode_hugepages(struct inode *inode,
loff_t lstart,
* the page, note PagePrivate which is used in case
* of error.
*/
- rsv_on_error = !PagePrivate(page);
remove_huge_page(page);
freed++;
if (!truncate_op) {
if (unlikely(hugetlb_unreserve_pages(inode,
next, next + 1, 1)))
- hugetlb_fix_reserve_counts(inode,
- rsv_on_error);
+ hugetlb_fix_reserve_counts(inode)
}
unlock_page(page);
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index c26d463..d2e0fc5 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -90,7 +90,7 @@ int dequeue_hwpoisoned_huge_page(struct page *page);
bool isolate_huge_page(struct page *page, struct list_head *list);
void putback_active_hugepage(struct page *page);
void free_huge_page(struct page *page);
-void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve);
+void hugetlb_fix_reserve_counts(struct inode *inode);
extern struct mutex *hugetlb_fault_mutex_table;
u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
struct vm_area_struct *vma,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 87e11d8..28a079a 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -567,13 +567,13 @@ retry:
* appear as a "reserved" entry instead of simply dangling with incorrect
* counts.
*/
-void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve)
+void hugetlb_fix_reserve_counts(struct inode *inode)
{
struct hugepage_subpool *spool = subpool_inode(inode);
long rsv_adjust;
rsv_adjust = hugepage_subpool_get_pages(spool, 1);
- if (restore_reserve && rsv_adjust) {
+ if (rsv_adjust) {
struct hstate *h = hstate_inode(inode);
hugetlb_acct_memory(h, 1);
Re: [RFC] remove unnecessary condition in remove_inode_hugepages
On 2016/9/24 1:19, Mike Kravetz wrote:
> On 09/22/2016 06:53 PM, zhong jiang wrote:
>> At present, we need to call hugetlb_fix_reserve_count when
>> hugetlb_unrserve_pages fails,
>> and PagePrivate will decide hugetlb reserves counts.
>>
>> we obtain the page from page cache. and use page both lock_page and
>> mutex_lock.
>> alloc_huge_page add page to page chace always hold lock page, then bail out
>> clearpageprivate
>> before unlock page.
>>
>> but I' m not sure it is right or I miss the points.
> Let me try to explain the code you suggest is unnecessary.
>
> The PagePrivate flag is used in huge page allocation/deallocation to
> indicate that the page was globally reserved. For example, in
> dequeue_huge_page_vma() there is this code:
>
> if (page) {
> if (avoid_reserve)
> break;
> if (!vma_has_reserves(vma, chg))
> break;
>
> SetPagePrivate(page);
> h->resv_huge_pages--;
> break;
> }
>
> and in free_huge_page():
>
> restore_reserve = PagePrivate(page);
> ClearPagePrivate(page);
> .
>
> .
> if (restore_reserve)
> h->resv_huge_pages++;
>
> This helps maintains the global huge page reserve count.
>
> In addition to the global reserve count, there are per VMA reservation
> structures. Unfortunately, these structures have different meanings
> depending on the context in which they are used.
>
> If there is a VMA reservation entry for a page, and the page has not
> been instantiated in the VMA this indicates there is a huge page reserved
> and the global resv_huge_pages count reflects that reservation. Even
> if a page was not reserved, a VMA reservation entry is added when a page
> is instantiated in the VMA.
>
> With that background, let's look at the existing code/proposed changes.
Clearly.
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index 4ea71eb..010723b 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -462,14 +462,12 @@ static void remove_inode_hugepages(struct inode
>> *inode, loff_t lstart,
>> * the page, note PagePrivate which is used in case
>> * of error.
>> */
>> - rsv_on_error = !PagePrivate(page);
> This rsv_on_error flag indicates that when the huge page was allocated,
yes
> it was NOT counted against the global reserve count. So, when
> remove_huge_page eventually calls free_huge_page(), the global count
> resv_huge_pages is not incremented. So far, no problem.
but the page comes from the page cache. if it is. it should implement
ClearPageprivate(page) when lock page. This condition always true.
The key point is why it need still check the PagePrivate(page) when page from
page cache and hold lock.
Thanks you
zhongjiang
>> remove_huge_page(page);
>> freed++;
>> if (!truncate_op) {
>> if (unlikely(hugetlb_unreserve_pages(inode,
>> next, next + 1, 1)))
> We now have this VERY unlikely situation that hugetlb_unreserve_pages fails.
> This means that the VMA reservation entry for the page was not removed.
> So, we are in a bit of a mess. The page has already been removed, but the
> VMA reservation entry can not. This LOOKS like there is a reservation for
> the page in the VMA reservation structure. But, the global count
> resv_huge_pages does not reflect this reservation.
>
> If we do nothing, when the VMA is eventually removed the VMA reservation
> structure will be completely removed and the global count resv_huge_pages
> will be decremented for each entry in the structure. Since, there is a
> VMA reservation entry without a corresponding global count, the global
> count will be one less than it should (will eventually go to -1).
>
> To 'fix' this, hugetlb_fix_reserve_counts is called. In this case, it will
> increment the global count so that it is consistent with the entries in
> the VMA reservation structure.
>
> This is all quite confusing and really unlikely to happen. I tried to
> explain in code comments:
>
> Before removing the page:
> /*
> * We must free the huge page and remove from p
Re: [RFC] remove unnecessary condition in remove_inode_hugepages
On 2016/9/25 8:06, Mike Kravetz wrote:
> On 09/23/2016 07:56 PM, zhong jiang wrote:
>> On 2016/9/24 1:19, Mike Kravetz wrote:
>>> On 09/22/2016 06:53 PM, zhong jiang wrote:
>>>> At present, we need to call hugetlb_fix_reserve_count when
>>>> hugetlb_unrserve_pages fails,
>>>> and PagePrivate will decide hugetlb reserves counts.
>>>>
>>>> we obtain the page from page cache. and use page both lock_page and
>>>> mutex_lock.
>>>> alloc_huge_page add page to page chace always hold lock page, then bail
>>>> out clearpageprivate
>>>> before unlock page.
>>>>
>>>> but I' m not sure it is right or I miss the points.
>>> Let me try to explain the code you suggest is unnecessary.
>>>
>>> The PagePrivate flag is used in huge page allocation/deallocation to
>>> indicate that the page was globally reserved. For example, in
>>> dequeue_huge_page_vma() there is this code:
>>>
>>> if (page) {
>>> if (avoid_reserve)
>>> break;
>>> if (!vma_has_reserves(vma, chg))
>>> break;
>>>
>>> SetPagePrivate(page);
>>> h->resv_huge_pages--;
>>> break;
>>> }
>>>
>>> and in free_huge_page():
>>>
>>> restore_reserve = PagePrivate(page);
>>> ClearPagePrivate(page);
>>> .
>>>
>>> .
>>> if (restore_reserve)
>>> h->resv_huge_pages++;
>>>
>>> This helps maintains the global huge page reserve count.
>>>
>>> In addition to the global reserve count, there are per VMA reservation
>>> structures. Unfortunately, these structures have different meanings
>>> depending on the context in which they are used.
>>>
>>> If there is a VMA reservation entry for a page, and the page has not
>>> been instantiated in the VMA this indicates there is a huge page reserved
>>> and the global resv_huge_pages count reflects that reservation. Even
>>> if a page was not reserved, a VMA reservation entry is added when a page
>>> is instantiated in the VMA.
>>>
>>> With that background, let's look at the existing code/proposed changes.
>> Clearly.
>>>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>>>> index 4ea71eb..010723b 100644
>>>> --- a/fs/hugetlbfs/inode.c
>>>> +++ b/fs/hugetlbfs/inode.c
>>>> @@ -462,14 +462,12 @@ static void remove_inode_hugepages(struct inode
>>>> *inode, loff_t lstart,
>>>> * the page, note PagePrivate which is used in case
>>>> * of error.
>>>> */
>>>> - rsv_on_error = !PagePrivate(page);
>>> This rsv_on_error flag indicates that when the huge page was allocated,
>>yes
>>> it was NOT counted against the global reserve count. So, when
>>> remove_huge_page eventually calls free_huge_page(), the global count
>>> resv_huge_pages is not incremented. So far, no problem.
>> but the page comes from the page cache. if it is. it should implement
>> ClearPageprivate(page) when lock page. This condition always true.
>>
>> The key point is why it need still check the PagePrivate(page) when page
>> from
>> page cache and hold lock.
> You are correct. My apologies for not seeing your point in the original
> post.
>
> When the huge page is added to the page cache (huge_add_to_page_cache),
> the Page Private flag will be cleared. Since this code
> (remove_inode_hugepages) will only be called for pages in the page cache,
> PagePrivate(page) will always be false.
>
> The comments in this area should be changed along with the code.
>
Thanks, I will resend the patch.
Re: [PATCH] fs: wipe off the compiler warn
On 2016/7/30 7:02, Andrew Morton wrote: > On Fri, 29 Jul 2016 22:46:39 +0800 zhongjiang wrote: > >> From: zhong jiang >> >> when compile the kenrel code, I happens to the following warn. >> fs/reiserfs/ibalance.c:1156:2: warning: ___new_insert_key___ may be used >> uninitialized in this function. >> memcpy(new_insert_key_addr, &new_insert_key, KEY_SIZE); >> ^ >> The patch just fix it to avoid the warn. >> >> Signed-off-by: zhong jiang >> --- >> fs/reiserfs/ibalance.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/reiserfs/ibalance.c b/fs/reiserfs/ibalance.c >> index b751eea..512ce95 100644 >> --- a/fs/reiserfs/ibalance.c >> +++ b/fs/reiserfs/ibalance.c >> @@ -818,7 +818,7 @@ int balance_internal(struct tree_balance *tb, >> int order; >> int insert_num, n, k; >> struct buffer_head *S_new; >> -struct item_head new_insert_key; >> +struct item_head uninitialized_var(new_insert_key); >> struct buffer_head *new_insert_ptr = NULL; >> struct item_head *new_insert_key_addr = insert_key; > How do we know this isn't a real bug? It isn't obvious to me that this > warning is a false positive. > > > . > yes ,it maybe a real bug, I will resend it in v2.
Re: [PATCH] x86/mm: Don't reenter flush_tlb_func_common()
On 2017/6/19 23:05, Andy Lutomirski wrote:
> On Mon, Jun 19, 2017 at 6:33 AM, zhong jiang wrote:
>> On 2017/6/19 12:48, Andy Lutomirski wrote:
>>> It was historically possible to have two concurrent TLB flushes
>>> targeting the same CPU: one initiated locally and one initiated
>>> remotely. This can now cause an OOPS in leave_mm() at
>>> arch/x86/mm/tlb.c:47:
>>>
>>> if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK)
>>> BUG();
>>>
>>> with this call trace:
>>> flush_tlb_func_local arch/x86/mm/tlb.c:239 [inline]
>>> flush_tlb_mm_range+0x26d/0x370 arch/x86/mm/tlb.c:317
>>>
>>> Without reentrancy, this OOPS is impossible: leave_mm() is only
>>> called if we're not in TLBSTATE_OK, but then we're unexpectedly
>>> in TLBSTATE_OK in leave_mm().
>>>
>>> This can be caused by flush_tlb_func_remote() happening between
>>> the two checks and calling leave_mm(), resulting in two consecutive
>>> leave_mm() calls on the same CPU with no intervening switch_mm()
>>> calls.
>>>
>>> We never saw this OOPS before because the old leave_mm()
>>> implementation didn't put us back in TLBSTATE_OK, so the assertion
>>> didn't fire.
>> HI, Andy
>>
>> Today, I see same OOPS in linux 3.4 stable. It prove that it indeed has
>> fired.
>>but It is rarely to appear. I review the code. I found the a issue.
>> when current->mm is NULL, leave_mm will be called. but it maybe in
>> TLBSTATE_OK, eg: unuse_mm call after task->mm = NULL , but before
>> enter_lazy_tlb.
>>
>>therefore, it will fire. is it right?
> Is there a code path that does this?
eg:
cpu1 cpu2
flush_tlb_page unuse_mm
current->mm
= NULL
current->mm == NULL
leave_mm (cpu_tlbstate.state is TLBSATATE_OK)
enter_lazy_tlb
I am not sure the above race whether exist or not. Do you point out the
problem if it is not existence? please
Thanks
zhongjiang
>
> Also, the IPI handler on 3.4 looks like this:
>
> if (f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
> if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
> if (f->flush_va == TLB_FLUSH_ALL)
> local_flush_tlb();
> else
> __flush_tlb_one(f->flush_va);
> } else
> leave_mm(cpu);
> }
>
> but leave_mm() checks the same condition (cpu_tlbstate.state, not
> current->mm). How is the BUG triggering?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org";> em...@kvack.org
>
>
Re: [PATCH] mm/vmalloc: a slight change of compare target in __insert_vmap_area()
On 2017/5/26 9:36, Wei Yang wrote: > On Thu, May 25, 2017 at 11:04:44AM +0800, zhong jiang wrote: >> I hit the overlap issue, but it is hard to reproduced. if you think it is >> safe. and the situation >> is not happen. AFAIC, it is no need to add the code. >> >> if you insist on the point. Maybe VM_WARN_ON is a choice. >> > Do you have some log to show the overlap happens? Hi wei cat /proc/vmallocinfo 0xf158-0xf160 524288 raw_dump_mem_write+0x10c/0x188 phys=8b901000 ioremap 0xf1638000-0xf163a0008192 mcss_pou_queue_init+0xa0/0x13c [mcss] phys=fc614000 ioremap 0xf528e000-0xf5292000 16384 n_tty_open+0x10/0xd0 pages=3 vmalloc 0xf500-0xf9001000 67112960 devm_ioremap+0x38/0x70 phys=4000 ioremap 0xfe001000-0xfe0020004096 iotable_init+0x0/0xc phys=20001000 ioremap 0xfe20-0xfe2010004096 iotable_init+0x0/0xc phys=1a00 ioremap 0xff10-0xff1010004096 iotable_init+0x0/0xc phys=2000a000 ioremap I hit the above issue, but the log no more useful info. it just is found by accident. and it is hard to reprodeced. no more info can be supported for further investigation. therefore, it is no idea for me. Thanks zhongjinag
is it a problem when cat /proc/pid/status
I want to know whether the NUMA bound memory node with CONFIG_CGROUPS related or associted with NUMA binding. I wrote an example test,the results are as follows. > euler-linux:/home/zhongjiang # numactl --membind=1 ./new & > [1] 6529 > euler-linux:/home/zhongjiang # ps -ef | grep new > root 6529 4483 0 14:40 pts/100:00:00 ./new > root 6556 4483 0 14:40 pts/100:00:00 grep new > euler-linux:/home/zhongjiang # cat /proc/6529/status > Name: new > State: S (sleeping) > Cpus_allowed: ff > Cpus_allowed_list: 0-23 > Mems_allowed: > ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,007f > Mems_allowed_list: 0-22 > voluntary_ctxt_switches:19 > nonvoluntary_ctxt_switches: 0 euler-linux:/home/zhongjiang # numactl --membind=3 ./new & [1] 9113 euler-linux:/home/zhongjiang # ps -ef | grep mew root 9140 8948 0 14:55 pts/100:00:00 grep mew euler-linux:/home/zhongjiang # ps -ef | grep new root 9113 8948 0 14:55 pts/100:00:00 ./new root 9209 8948 0 14:55 pts/100:00:00 grep new euler-linux:/home/zhongjiang # cat /proc/9113/status Name: new State: S (sleeping) Tgid: 9113 Ngid: 0 Pid:9113 PPid: 8948 .. Cpus_allowed: ff Cpus_allowed_list: 0-23 Mems_allowed: ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,007f Mems_allowed_list: 0-22 voluntary_ctxt_switches:26 nonvoluntary_ctxt_switches: 0 Through the comparison of the above, I can find Whatever I use node for binding, the mems_allowed fields has been no change -- 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/
some problems about kasan
1、 I feel confused about one of the cases when testing the cases kasan can
solve . the function come from the kernel in the /lib/test_kasan.c.
static noinline void __init kmalloc_uaf2(void)
{
char *ptr1, *ptr2;
size_t size = 43;
pr_info("use-after-free after another kmalloc\n");
ptr1 = kmalloc(size, GFP_KERNEL);
if (!ptr1) {
pr_err("Allocation failed\n");
return;
}
kfree(ptr1);
ptr2 = kmalloc(size, GFP_KERNEL);
if (!ptr2) {
pr_err("Allocation failed\n");
return;
}
ptr1[40] = 'x';
kfree(ptr2);
}
In the above function, the point ptr1 are probably the same as the ptr2 . so
the error not certain to occur.
2、Is the stack local variable out of bound access set by the GCC ? I don't see
any operate in the kernel
3、I want to know that the global variable size include redzone is allocated by
the module_alloc().
--
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/
memory-hotplug have such a problem.
when I test the memory online or offline, I find the problem that memory online always return an error. I peform the operation as follow: cd /sys/device/system/memory/ echo online > state it always print "-bash: echo: write error: Operation not permitted". By my alalysis, I find that the error happen in the memory_notifier(MEM_GOING_ONLINE,&arg) in the function(online_pages()). The problem occur in the latest kernel versin (linux 4.3.0 -rc4) , but In the lower version (linux 3.10) is correct. I think it may be caused by the confilct. Thanks zhongjiang -- 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: [Question] A novel case happened when using mempool allocate memory.
On 2018/8/2 21:31, Matthew Wilcox wrote:
> On Thu, Aug 02, 2018 at 02:22:03PM +0800, zhong jiang wrote:
>> On 2018/8/1 23:37, Matthew Wilcox wrote:
>>> Please post the code.
>> when module is loaded. we create the specific mempool. The code flow is as
>> follows.
> I actually meant "post the code you are testing", not "write out some
> pseudocode".
>
> .
>
The source code is as follow about mempool utility.
**
* @brief 元素的类型顺序与 @smio_mem_type_t的定义顺序一致
*/
static smio_mem_mng_t g_smio_mem[] =
{
{
.name = "MEDIA_INFO",
.min_pool_size = 128,
.item_size = sizeof(smio_media_info_t),
.slab_cache = NULL,
},
{
.name = "DSW_IO_REQ",
.min_pool_size = 1024,
.item_size = sizeof(dsw_io_req_t),
.slab_cache = NULL,
},
{
.name = "DSW_IO_PAGE",
.min_pool_size = 1024,
.item_size = sizeof(dsw_page_t) * DSW_MAX_PAGE_PER_REQ,
.slab_cache = NULL,
},
{
.name = "32_ARRAY",
.min_pool_size = 1024,
.item_size = sizeof(void *) * 32,
.slab_cache = NULL,
},
{
.name = "SCSI_SENSE_BUF",
.min_pool_size = 1024,
.item_size = sizeof(char) * SCSI_SENSE_BUFFERSIZE,
.slab_cache = NULL,
},
};
/**
* @brief 申请数据类型内存
*
* @param id 申请者模块ID
* @param type 申请内存的类型
*
* @return 成功返回内存块的首地址;失败返回NULL
*/
void *smio_mem_alloc(smio_module_id_t id, smio_mem_type_t type)
{
void *m = NULL;
smio_mem_mng_t *pool_mng = NULL;
SMIO_ASSERT_RETURN(id < SMIO_MOD_ID_BUTT, NULL);
SMIO_ASSERT_RETURN(type < SMIO_MEM_TYPE_BUTT, NULL);
pool_mng = &g_smio_mem[type];
SMIO_LOG_DEBUG("alloc %s, size: %d\n", pool_mng->name, pool_mng->item_size);
m = mempool_alloc(pool_mng->pool, GFP_KERNEL);
if (NULL == m)
{
return NULL;
}
memset(m, 0, pool_mng->item_size);
atomic_inc(&pool_mng->statistics[id]);
return m;
}
EXPORT_SYMBOL(smio_mem_alloc);
/**
* @brief 释放内存块
*
* @param id 申请者的模块ID
* @param type 内存块的类型
* @param m 内在的首地址
*/
void smio_mem_free(smio_module_id_t id, smio_mem_type_t type, void *m)
{
smio_mem_mng_t *pool_mng = NULL;
SMIO_ASSERT(NULL != m);
SMIO_ASSERT(id < SMIO_MOD_ID_BUTT);
SMIO_ASSERT(type < SMIO_MEM_TYPE_BUTT);
pool_mng = &g_smio_mem[type];
mempool_free(m, pool_mng->pool);
atomic_dec(&pool_mng->statistics[id]);
}
EXPORT_SYMBOL(smio_mem_free);
/**
* @brief 创建管理内在池
*
* @param pool_mng 内存类型管理结构
*
* @return 成功返回@SMIO_OK;失败返回@SMIO_ERR
*/
static int smio_mem_pool_create(smio_mem_mng_t *pool_mng)
{
int i;
SMIO_ASSERT_RETURN(NULL != pool_mng, SMIO_ERR);
pool_mng->slab_cache = kmem_cache_create(pool_mng->name,
pool_mng->item_size, 0, 0, NULL);
if (SMIO_IS_ERR_OR_NULL(pool_mng->slab_cache))
{
SMIO_LOG_ERR("kmem_cache_create for %s failed\n", pool_mng->name);
return SMIO_ERR;
}
pool_mng->pool = mempool_create(pool_mng->min_pool_size, mempool_alloc_slab,
mempool_free_slab, pool_mng->slab_cache);
if (NULL == pool_mng->pool)
{
SMIO_LOG_ERR("pool create for %s failed\n", pool_mng->name);
kmem_cache_destroy(pool_mng->slab_cache);
return SMIO_ERR;
}
for (i = 0; i < SMIO_MOD_ID_BUTT; i++)
{
atomic_set(&pool_mng->statistics[i], 0);
}
return SMIO_OK;
}
/**
* @brief 清除内存池
*
* @param pool_mng 所要清除的内存池
*/
void smio_mem_pool_destroy(smio_mem_mng_t *pool_mng)
{
SMIO_ASSERT(NULL != pool_mng);
if (NULL != pool_mng->pool)
{
mempool_destroy(pool_mng->pool);
pool_mng->pool = NULL;
}
if (NULL != pool_mng->slab_cache)
{
kmem_cache_destroy(pool_mng->slab_cache);
pool_mng->slab_cache = NULL;
}
}
/**
* @brief 内存管理单元初始化
*
* @return 成功返回@SMIO_OK;失败返回@SMIO_ERR
*/
int smio_mem_init(void)
{
int i;
int pool_num = (int) SMIO_ARRAY_SIZE(g_smio_mem);
int ret = SMIO_OK;
bool free = SMIO_FALSE;
for (i = 0; i < pool_num; i++)
{
SMIO_LOG_INFO("memory of %s initialize, min_pool_size: %d, item_size: %d\n",
g_smio_mem[i].name, g_smio_mem[i].min_pool_size, g_smio_mem[i].item_size);
if (SMIO_OK != smio_mem_pool_create(&g_smio_mem[i]))
{
SMIO_LOG_ERR("memory of %s initialize failed\n", g_smio_mem[i].name);
ret = SMIO_ERR;
free = SMIO_TRUE;
break;
}
}
/* clean if smio_mem_pool_create failed*/
while ((SMIO_TRUE == free) && (--i >= 0))
{
smio_mem_pool_destroy(&g_smio_mem[i]);
}
return ret;
}
/**
* @brief 内存管理模块清除退出
*/
void smio_mem_exit(void)
{
int i;
int pool_num = (int) SMIO_ARRAY_SIZE(g_smio_mem);
for (i = 0; i < pool_num; i++)
{
smio_mem_pool_destroy(&g_smio_mem[i]);
}
}
Re: [PATCH linux-next] driver/gpu: Fix mismatch in funciton argument type
On 2018/7/22 13:41, zhong jiang wrote:
> Fix the following warning:
>
> drivers/gpu/drm/nouveau/dispnv50/wndw.c:570:1: error: symbol 'nv50_wndw_new_'
> redeclared with different type
> (originally declared at drivers/gpu/drm/nouveau/dispnv50/wndw.h:39) -
> incompatible argument 7 (different signedness)
>
> Signed-off-by: zhong jiang
> ---
> drivers/gpu/drm/nouveau/dispnv50/wndw.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> index b0b6428..0770683 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> @@ -38,8 +38,8 @@ struct nv50_wndw {
>
> int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *,
> enum drm_plane_type, const char *name, int index,
> -const u32 *format, enum nv50_disp_interlock_type,
> -u32 interlock_data, u32 heads, struct nv50_wndw **);
> +const u32 *format, u32 heads, enum nv50_disp_interlock_type,
> +u32 interlock_data, struct nv50_wndw **);
> void nv50_wndw_init(struct nv50_wndw *);
> void nv50_wndw_fini(struct nv50_wndw *);
> void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock,
ping ?
[PATCH] sched/topology: Make some local variable static
Fix the following warning. kernel/sched/topology.c:10:15: warning: symbol 'sched_domains_tmpmask' was not declared. Should it be static? kernel/sched/topology.c:11:15: warning: symbol 'sched_domains_tmpmask2' was not declared. Should it be static? Signed-off-by: zhong jiang --- kernel/sched/topology.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index 56a0fed..cf3d756 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c @@ -7,8 +7,8 @@ DEFINE_MUTEX(sched_domains_mutex); /* Protected by sched_domains_mutex: */ -cpumask_var_t sched_domains_tmpmask; -cpumask_var_t sched_domains_tmpmask2; +static cpumask_var_t sched_domains_tmpmask; +static cpumask_var_t sched_domains_tmpmask2; #ifdef CONFIG_SCHED_DEBUG -- 1.7.12.4
[PATCH] kernel/kthread: Make function __kthread_queue_delayed_work static
Fix the following warning.
kernel/kthread.c:859:6: warning: symbol '__kthread_queue_delayed_work' was not
declared. Should it be static?
Signed-off-by: zhong jiang
---
kernel/kthread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 087d18d..3c661a0 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -856,7 +856,7 @@ void kthread_delayed_work_timer_fn(struct timer_list *t)
}
EXPORT_SYMBOL(kthread_delayed_work_timer_fn);
-void __kthread_queue_delayed_work(struct kthread_worker *worker,
+static void __kthread_queue_delayed_work(struct kthread_worker *worker,
struct kthread_delayed_work *dwork,
unsigned long delay)
{
--
1.7.12.4
[PATCH] ext4/mballoc: Remove unneeded variable "err"
The err is not used after initalization. So just remove the variable. Signed-off-by: zhong jiang --- fs/ext4/mballoc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 8b24d3d..e29fce2 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -3801,7 +3801,6 @@ static int ext4_mb_new_preallocation(struct ext4_allocation_context *ac) ext4_group_t group; ext4_grpblk_t bit; unsigned long long grp_blk_start; - int err = 0; int free = 0; BUG_ON(pa->pa_deleted == 0); @@ -3842,7 +3841,7 @@ static int ext4_mb_new_preallocation(struct ext4_allocation_context *ac) } atomic_add(free, &sbi->s_mb_discarded); - return err; + return 0; } static noinline_for_stack int -- 1.7.12.4
Re: [PATCH] ext4/mballoc: Remove unneeded variable "err"
On 2018/8/5 5:24, Theodore Y. Ts'o wrote: > On Sat, Aug 04, 2018 at 07:04:56PM +0800, zhong jiang wrote: >> The err is not used after initalization. So just remove the variable. >> >> Signed-off-by: zhong jiang > I'll apply this patch, but how did you generate the diff? The > function name here is all wrong: > >> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c >> index 8b24d3d..e29fce2 100644 >> --- a/fs/ext4/mballoc.c >> +++ b/fs/ext4/mballoc.c >> @@ -3801,7 +3801,6 @@ static int ext4_mb_new_preallocation(struct >> ext4_allocation_context *ac) >^ > > The lines in question are from ext4_mb_release_inode_pa(), *not* > ext4_mb_new_preallocation(). So when I inspected the patch visually, > my first reaction was, "there's no way this patch would apply". > > But then I looked at the C code changed by the patch, and I was > surprised to see that it applied correctly in a completely different > function, and when I regenerated the patch, the line numbers matched > yours --- so the only thing "wrong" in your patch is the function name. > > So that raises the question --- how did you create the diff in this-rc > patch? What version of git? And what version of the kernel? > > Regards, > > - Ted > > . > I create the diff in this patch again ,but get the same result . the git version is git version 1.7.12.4 but I create the diff in git version 1.8.3.1. It shows the correct function name. The kernel version is 4.18-rc7. I will upgrade the git version. Thanks, zhong jiang
Re: [PATCH] kernel/kthread: Make function __kthread_queue_delayed_work static
On 2018/8/3 21:21, Steven Rostedt wrote:
> On Fri, 3 Aug 2018 20:40:02 +0800
> zhong jiang wrote:
>
>> Fix the following warning.
>>
>> kernel/kthread.c:859:6: warning: symbol '__kthread_queue_delayed_work' was
>> not declared. Should it be static?
>
> This should go via the trivial tree.
>
>> Signed-off-by: zhong jiang
>> ---
>> kernel/kthread.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/kthread.c b/kernel/kthread.c
>> index 087d18d..3c661a0 100644
>> --- a/kernel/kthread.c
>> +++ b/kernel/kthread.c
>> @@ -856,7 +856,7 @@ void kthread_delayed_work_timer_fn(struct timer_list *t)
>> }
>> EXPORT_SYMBOL(kthread_delayed_work_timer_fn);
>>
>> -void __kthread_queue_delayed_work(struct kthread_worker *worker,
>> +static void __kthread_queue_delayed_work(struct kthread_worker *worker,
>>struct kthread_delayed_work *dwork,
>>unsigned long delay)
> Although, one could say you should align the parameters too, but that's
> not a big deal. Anyway...
>
> Reviewed-by: Steven Rostedt (VMware)
Thanks , I will notice for next time.
> -- Steve
>
>> {
>
> .
>
[PATCH] drivers/staging: Remove some unneeded semicolon
That semicolons are unneeded, JUst remove them.
Signed-off-by: zhong jiang
---
drivers/staging/erofs/unzip_vle.c | 2 +-
drivers/staging/gasket/gasket_interrupt.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 1030ca5..e5ceb68 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1016,7 +1016,7 @@ static void z_erofs_vle_unzip_all(struct super_block *sb,
owned = READ_ONCE(grp->next);
z_erofs_vle_unzip(sb, grp, page_pool);
- };
+ }
}
static void z_erofs_vle_unzip_wq(struct work_struct *work)
diff --git a/drivers/staging/gasket/gasket_interrupt.c
b/drivers/staging/gasket/gasket_interrupt.c
index 09c3d07..1cfbc12 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -386,7 +386,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev,
const char *name,
"Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
ret = -EINVAL;
- };
+ }
if (ret) {
/* Failing to setup interrupts will cause the device to report
@@ -445,7 +445,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
"Cannot handle unsupported interrupt type %d\n",
gasket_dev->interrupt_data->type);
ret = -EINVAL;
- };
+ }
if (ret) {
/* Failing to setup MSIx will cause the device
@@ -493,7 +493,7 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
dev_dbg(gasket_dev->dev,
"Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
- };
+ }
kfree(interrupt_data->interrupt_counts);
kfree(interrupt_data->eventfd_ctxs);
--
1.7.12.4
Re: [PATCH] drivers/staging: Remove some unneeded semicolon
On 2018/8/6 9:27, Gao Xiang wrote: > Hi Jiang, > > On 2018/8/5 21:57, zhong jiang wrote: >> That semicolons are unneeded, JUst remove them. >> >> Signed-off-by: zhong jiang > Thanks for your patch. Since erofs and gasket are different feature, it is > better to seperate into two patches. > and could you please cc linux-erofs mailing list > as well? > > Yes, there is an extra semicolon in z_erofs_vle_unzip_all, it was reported by > Julia Lawall several days ago. > Actually, there is a patch in linux-erofs mailing list, but it seems that > Chao hasn't reviewed it yet... > > https://lists.ozlabs.org/pipermail/linux-erofs/2018-August/000303.html > > I will add Signed-off-by: zhong jiang to the original > patch if if you don't mind, do you? do it. Thanks. Then I will just resend the gasket feature separately. Thanks, zhong jiang > Thanks, > Gao Xiang > > . >
Re: [PATCH] net/bridge/br_multicast: remove redundant variable "err"
On 2018/8/6 8:33, David Miller wrote: > From: zhong jiang > Date: Sun, 5 Aug 2018 22:18:43 +0800 > >> @@ -1797,7 +1795,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br, >> struct sk_buff *skb_trimmed = NULL; >> const unsigned char *src; >> struct igmphdr *ih; >> -int err; >> +int err = 0; >> >> err = ip_mc_check_igmp(skb, &skb_trimmed); > The initialization of err to '0' is unnecessary, it gets assigned on > the very next line. > > . > That's right. I will drop that change and repost. Thanks, zhong jiang
Re: [PATCH 0/3] Remove uneeded variable "err"
On 2018/8/6 3:14, Nikolay Borisov wrote: > > On 5.08.2018 18:02, zhong jiang wrote: >> zhong jiang (3): >> fs/btrfs/disk-io: Remove unneeded variable "err" >> fs/btrfs/extent-tree: remove redudant variable "err" >> fs/btrfs/tree-log: remove the unneeded variable "err" >> >> fs/btrfs/disk-io.c | 12 >> fs/btrfs/extent-tree.c | 6 ++ >> fs/btrfs/tree-log.c| 5 + >> fs/btrfs/tree-log.h| 2 +- >> 4 files changed, 8 insertions(+), 17 deletions(-) >> > > NAK on the whole series on the basis of using a bulk commit message > which doesn't really describe the changes of each individual commit. If > you want those changes to eventually be merged resubmit the series with > individual reasoning for every commit. > > It's sorry for using a bulk commit message. I will repost with a patch because it solves the same issue. Thanks, zhong jiang
Re: [PATCH 1/3] fs/btrfs/disk-io: Remove unneeded variable "err"
On 2018/8/5 23:27, Joe Perches wrote:
> On Sun, 2018-08-05 at 23:02 +0800, zhong jiang wrote:
>> The err is not used after initalization, So remove it and make
>> the function to be void function.
> []
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> []
>> @@ -4193,7 +4192,6 @@ static int btrfs_destroy_delayed_refs(struct
>> btrfs_transaction *trans,
>> if (atomic_read(&delayed_refs->num_entries) == 0) {
>> spin_unlock(&delayed_refs->lock);
>> btrfs_info(fs_info, "delayed_refs has NO entry");
>> -return ret;
> Think a little more about this please.
> This is not a sensible removal.
I am sorry for stupid mistake. I will repost.
Thanks,
zhong jiang
>> }
>>
>> while ((node = rb_first(&delayed_refs->href_root)) != NULL) {
>
> .
>
[PATCH] staging: gasket: remove some extra semicolon
That semicolons are unneeded, Just remove them.
Signed-off-by: zhong jiang
---
drivers/staging/gasket/gasket_interrupt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c
b/drivers/staging/gasket/gasket_interrupt.c
index 09c3d07..1cfbc12 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -386,7 +386,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev,
const char *name,
"Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
ret = -EINVAL;
- };
+ }
if (ret) {
/* Failing to setup interrupts will cause the device to report
@@ -445,7 +445,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
"Cannot handle unsupported interrupt type %d\n",
gasket_dev->interrupt_data->type);
ret = -EINVAL;
- };
+ }
if (ret) {
/* Failing to setup MSIx will cause the device
@@ -493,7 +493,7 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
dev_dbg(gasket_dev->dev,
"Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
- };
+ }
kfree(interrupt_data->interrupt_counts);
kfree(interrupt_data->eventfd_ctxs);
--
1.7.12.4
[PATCH v2] drivers/thermal/tegra: Fix a double free on the device node
The function 'for_each_child_of_node' iterates over the node list by
dropping the of_node reference of the previous node.
Calling of_node_put() on the iterator is pointless and leads to an
inconsistent refcounting in addition to a double free. Remove it.
Acked-by: Jon Hunter
Signed-off-by: zhong jiang
---
v1->v2: According to Daniel's suggestion. I modify the subject and commit log.
drivers/thermal/tegra/soctherm.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index ed28110..3aa55c9 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -980,7 +980,6 @@ static void soctherm_init_hw_throt_cdev(struct
platform_device *pdev)
tcd = thermal_of_cooling_device_register(np_stcc,
(char *)name, ts,
&throt_cooling_ops);
- of_node_put(np_stcc);
if (IS_ERR_OR_NULL(tcd)) {
dev_err(dev,
"throttle-cfg: %s: failed to register cooling
device\n",
--
1.7.12.4
[PATCH] drivers/block/mtip32xx: remove the null check for debugfs_remove_recursive
debugfs_remove_recursive has taken null pointer into account. So it is
safe to drop the null check before calling the funtion.
Signed-off-by: zhong jiang
---
drivers/block/mtip32xx/mtip32xx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c
b/drivers/block/mtip32xx/mtip32xx.c
index 2b6d6bc..d0666f5c 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -2574,8 +2574,7 @@ static int mtip_hw_debugfs_init(struct driver_data *dd)
static void mtip_hw_debugfs_exit(struct driver_data *dd)
{
- if (dd->dfs_node)
- debugfs_remove_recursive(dd->dfs_node);
+ debugfs_remove_recursive(dd->dfs_node);
}
/*
--
1.7.12.4
[PATCH] drivers/block/aoe/aoedev: NULL check is not needed for mempool_destroy
mempool_destroy has taken the null pointer into account. So it is safe to remove the null check. Signed-off-by: zhong jiang --- drivers/block/aoe/aoedev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c index 697f735..41060e9 100644 --- a/drivers/block/aoe/aoedev.c +++ b/drivers/block/aoe/aoedev.c @@ -284,8 +284,8 @@ e = t + d->ntargets; for (; t < e && *t; t++) freetgt(d, *t); - if (d->bufpool) - mempool_destroy(d->bufpool); + + mempool_destroy(d->bufpool); skbpoolfree(d); minor_free(d->sysminor); -- 1.7.12.4
[PATCH] drivers/block/drbd: remove the null check for kmem_cache_destroy
kmem_cache_destroy has taken null pointer into account. So it is safe to drop the null check before calling the function. Signed-off-by: zhong jiang --- drivers/block/drbd/drbd_main.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c index a80809b..ef8212a 100644 --- a/drivers/block/drbd/drbd_main.c +++ b/drivers/block/drbd/drbd_main.c @@ -2103,14 +2103,10 @@ static void drbd_destroy_mempools(void) mempool_exit(&drbd_md_io_page_pool); mempool_exit(&drbd_ee_mempool); mempool_exit(&drbd_request_mempool); - if (drbd_ee_cache) - kmem_cache_destroy(drbd_ee_cache); - if (drbd_request_cache) - kmem_cache_destroy(drbd_request_cache); - if (drbd_bm_ext_cache) - kmem_cache_destroy(drbd_bm_ext_cache); - if (drbd_al_ext_cache) - kmem_cache_destroy(drbd_al_ext_cache); + kmem_cache_destroy(drbd_ee_cache); + kmem_cache_destroy(drbd_request_cache); + kmem_cache_destroy(drbd_bm_ext_cache); + kmem_cache_destroy(drbd_al_ext_cache); drbd_ee_cache= NULL; drbd_request_cache = NULL; -- 1.7.12.4
[PATCH] drivers/thermal/tegra: fix a doule free devce node
Device node iterators will get the return node. Meawhile, It is
also put the previous device node. An explicit put will cause
a double put.
Signed-off-by: zhong jiang
---
drivers/thermal/tegra/soctherm.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index ed28110..3aa55c9 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -980,7 +980,6 @@ static void soctherm_init_hw_throt_cdev(struct
platform_device *pdev)
tcd = thermal_of_cooling_device_register(np_stcc,
(char *)name, ts,
&throt_cooling_ops);
- of_node_put(np_stcc);
if (IS_ERR_OR_NULL(tcd)) {
dev_err(dev,
"throttle-cfg: %s: failed to register cooling
device\n",
--
1.7.12.4
[PATCH 1/2] arm64/kprobes: remove an extra semicolon in arch_prepare_kprobe
There is an extra semicolon in arch_prepare_kprobe, remove it. Signed-off-by: zhong jiang --- arch/arm64/kernel/probes/kprobes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index e78c3ef..9b65132 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -107,7 +107,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p) if (!p->ainsn.api.insn) return -ENOMEM; break; - }; + } /* prepare the instruction */ if (p->ainsn.api.insn) -- 1.7.12.4
[PATCH 2/2] arm64:guest: remove some extra semicolon in kvm_target_cpu
There are some extra semicolon in kvm_target_cpu, remove it.
Signed-off-by: zhong jiang
---
arch/arm64/kvm/guest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 07256b0..a74f84d 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -338,15 +338,15 @@ int __attribute_const__ kvm_target_cpu(void)
return KVM_ARM_TARGET_CORTEX_A53;
case ARM_CPU_PART_CORTEX_A57:
return KVM_ARM_TARGET_CORTEX_A57;
- };
+ }
break;
case ARM_CPU_IMP_APM:
switch (part_number) {
case APM_CPU_PART_POTENZA:
return KVM_ARM_TARGET_XGENE_POTENZA;
- };
+ }
break;
- };
+ }
/* Return a default generic target */
return KVM_ARM_TARGET_GENERIC_V8;
--
1.7.12.4
[PATCH 0/2] arm64: remove some extra semicolon
There are some extra semicolon in arm64 architecture. Just remove them. zhong jiang (2): arm64/kprobes: remove an extra semicolon in arch_prepare_kprobe arm64:guest: remove some extra semicolon in kvm_target_cpu arch/arm64/kernel/probes/kprobes.c | 2 +- arch/arm64/kvm/guest.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) -- 1.7.12.4
[PATCH] arm:pmsa-v8: remove some extra semicolon
There are some extra semicolon in pmsa-v8.c. So just remove them. I detect the issue with the help of Coccinelle. Signed-off-by: zhong jiang --- arch/arm/mm/pmsa-v8.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mm/pmsa-v8.c b/arch/arm/mm/pmsa-v8.c index 617a83d..0d7d5fb 100644 --- a/arch/arm/mm/pmsa-v8.c +++ b/arch/arm/mm/pmsa-v8.c @@ -165,7 +165,7 @@ static int __init pmsav8_setup_ram(unsigned int number, phys_addr_t start,phys_a return -EINVAL; bar = start; - lar = (end - 1) & ~(PMSAv8_MINALIGN - 1);; + lar = (end - 1) & ~(PMSAv8_MINALIGN - 1); bar |= PMSAv8_AP_PL1RW_PL0RW | PMSAv8_RGN_SHARED; lar |= PMSAv8_LAR_IDX(PMSAv8_RGN_NORMAL) | PMSAv8_LAR_EN; @@ -181,7 +181,7 @@ static int __init pmsav8_setup_io(unsigned int number, phys_addr_t start,phys_ad return -EINVAL; bar = start; - lar = (end - 1) & ~(PMSAv8_MINALIGN - 1);; + lar = (end - 1) & ~(PMSAv8_MINALIGN - 1); bar |= PMSAv8_AP_PL1RW_PL0RW | PMSAv8_RGN_SHARED | PMSAv8_BAR_XN; lar |= PMSAv8_LAR_IDX(PMSAv8_RGN_DEVICE_nGnRnE) | PMSAv8_LAR_EN; -- 1.7.12.4
[PATCH] ia64:tioce_provider: Use kmemdup rather than implement the function
The kmemdup has implemented the function that kmalloc() + memcpy will do. So just replace them to make the code concise. Signed-off-by: zhong jiang --- arch/ia64/sn/pci/tioce_provider.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/ia64/sn/pci/tioce_provider.c b/arch/ia64/sn/pci/tioce_provider.c index 3bd9abc..9ba61bc 100644 --- a/arch/ia64/sn/pci/tioce_provider.c +++ b/arch/ia64/sn/pci/tioce_provider.c @@ -1000,11 +1000,11 @@ * Allocate kernel bus soft and copy from prom. */ - tioce_common = kzalloc(sizeof(struct tioce_common), GFP_KERNEL); + tioce_common = kmemdup(prom_bussoft, sizeof(struct tioce_common), + GFP_KERNEL); if (!tioce_common) return NULL; - memcpy(tioce_common, prom_bussoft, sizeof(struct tioce_common)); tioce_common->ce_pcibus.bs_base = (unsigned long) ioremap(REGION_OFFSET(tioce_common->ce_pcibus.bs_base), sizeof(struct tioce_common)); -- 1.7.12.4
[PATCH] arm:pm: Use kmemdup to replace duplicating its implementation
kmemdup is better than kmalloc() + memcpy(), and we do not like
open code. So just use kmemdup instead.
Signed-off-by: zhong jiang
---
arch/arm/mach-lpc32xx/pm.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-lpc32xx/pm.c b/arch/arm/mach-lpc32xx/pm.c
index 6247157..6eefff8 100644
--- a/arch/arm/mach-lpc32xx/pm.c
+++ b/arch/arm/mach-lpc32xx/pm.c
@@ -86,7 +86,8 @@ static int lpc32xx_pm_enter(suspend_state_t state)
void *iram_swap_area;
/* Allocate some space for temporary IRAM storage */
- iram_swap_area = kmalloc(lpc32xx_sys_suspend_sz, GFP_KERNEL);
+ iram_swap_area = kmemdup((void *)TEMP_IRAM_AREA,
+ lpc32xx_sys_suspend_sz, GFP_KERNEL);
if (!iram_swap_area) {
printk(KERN_ERR
"PM Suspend: cannot allocate memory to save portion "
@@ -94,10 +95,6 @@ static int lpc32xx_pm_enter(suspend_state_t state)
return -ENOMEM;
}
- /* Backup a small area of IRAM used for the suspend code */
- memcpy(iram_swap_area, (void *) TEMP_IRAM_AREA,
- lpc32xx_sys_suspend_sz);
-
/*
* Copy code to suspend system into IRAM. The suspend code
* needs to run from IRAM as DRAM may no longer be available
--
1.7.12.4
[PATCH] staging:pci-mt7621: Use PTR_ERR_OR_ZERO to replace the open code
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So just replace them rather than duplicating its implement. Signed-off-by: zhong jiang --- drivers/staging/mt7621-pci/pci-mt7621.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index a49e279..9a8ff1d 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -403,10 +403,8 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) } pcie->base = devm_ioremap_resource(dev, ®s); - if (IS_ERR(pcie->base)) - return PTR_ERR(pcie->base); - return 0; + return PTR_ERR_OR_ZERO(pcie->base); } static int mt7621_pcie_request_resources(struct mt7621_pcie *pcie, -- 1.7.12.4
Re: [PATCH] staging:pci-mt7621: Use PTR_ERR_OR_ZERO to replace the open code
On 2018/8/13 19:26, Sergio Paracuellos wrote: > On Mon, Aug 13, 2018 at 12:54 PM, zhong jiang wrote: >> PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So >> just replace them rather than duplicating its implement. >> >> Signed-off-by: zhong jiang >> --- >> drivers/staging/mt7621-pci/pci-mt7621.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c >> b/drivers/staging/mt7621-pci/pci-mt7621.c >> index a49e279..9a8ff1d 100644 >> --- a/drivers/staging/mt7621-pci/pci-mt7621.c >> +++ b/drivers/staging/mt7621-pci/pci-mt7621.c >> @@ -403,10 +403,8 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie >> *pcie) >> } >> >> pcie->base = devm_ioremap_resource(dev, ®s); >> - if (IS_ERR(pcie->base)) >> - return PTR_ERR(pcie->base); >> >> - return 0; >> + return PTR_ERR_OR_ZERO(pcie->base); >> } > Hi Zhong, > > Thanks for this. This driver is in the middle of cleanup and this > function is not doing all the DT parse > yet, so this patch looks good but after applying this series (if they > work and are good enough to be applied) > has no sense. > > See: > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124890.html > > Best regards, > Sergio Paracuellos Thank you for let me know that. Sincerely, zhong jiang >> static int mt7621_pcie_request_resources(struct mt7621_pcie *pcie, >> -- >> 1.7.12.4 >> > . >
[PATCH 2/2] phy:phy-lantiq-rcu-usb2: Use PTR_ERR_OR_ZERO to replace the open code
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So just replace them rather than duplicating its implement. Signed-off-by: zhong jiang --- drivers/phy/lantiq/phy-lantiq-rcu-usb2.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c b/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c index 986224f..a918c5b 100644 --- a/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c +++ b/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c @@ -196,10 +196,8 @@ static int ltq_rcu_usb2_of_parse(struct ltq_rcu_usb2_priv *priv, } priv->phy_reset = devm_reset_control_get_optional(dev, "phy"); - if (IS_ERR(priv->phy_reset)) - return PTR_ERR(priv->phy_reset); - return 0; + return PTR_ERR_OR_ZERO(priv->phy_reset); } static int ltq_rcu_usb2_phy_probe(struct platform_device *pdev) -- 1.7.12.4
[PATCH 0/2] phy: Use PTR_ERR_OR_ZERO to replace the open code
The issue is detected with the help of Coccinelle. zhong jiang (2): phy:phy-brcm-us: Use PTR_ERR_OR_ZERO to replace the open code phy:phy-lantiq-rcu-usb2: Use PTR_ERR_OR_ZERO to replace the open code drivers/phy/broadcom/phy-brcm-usb.c | 4 +--- drivers/phy/lantiq/phy-lantiq-rcu-usb2.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) -- 1.7.12.4
[PATCH 1/2] phy:phy-brcm-us: Use PTR_ERR_OR_ZERO to replace the open code
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So just replace them rather than duplicating its implement. Signed-off-by: zhong jiang --- drivers/phy/broadcom/phy-brcm-usb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c index d1dab36..f59b1dc 100644 --- a/drivers/phy/broadcom/phy-brcm-usb.c +++ b/drivers/phy/broadcom/phy-brcm-usb.c @@ -372,10 +372,8 @@ static int brcm_usb_phy_probe(struct platform_device *pdev) clk_disable(priv->usb_30_clk); phy_provider = devm_of_phy_provider_register(dev, brcm_usb_phy_xlate); - if (IS_ERR(phy_provider)) - return PTR_ERR(phy_provider); - return 0; + return PTR_ERR_OR_ZERO(phy_provider); } #ifdef CONFIG_PM_SLEEP -- 1.7.12.4
[PATCH 0/2] rtc: Use PTR_ERR_OR_ZERO to replace the open code
The issue is detected with the help of Coccinelle. zhong jiang (2): rtc:rtc-digicolor: Use PTR_ERR_OR_ZERO to replace the open code rtc:rtc-ds1347: Use PTR_ERR_OR_ZERO to replace the open code drivers/rtc/rtc-digicolor.c | 4 +--- drivers/rtc/rtc-ds1347.c| 5 + 2 files changed, 2 insertions(+), 7 deletions(-) -- 1.7.12.4
[PATCH 1/2] rtc:rtc-digicolor: Use PTR_ERR_OR_ZERO to replace the open code
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So
just replace them rather than duplicating its implement.
Signed-off-by: zhong jiang
---
drivers/rtc/rtc-digicolor.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
index b253bf1..fd6850c 100644
--- a/drivers/rtc/rtc-digicolor.c
+++ b/drivers/rtc/rtc-digicolor.c
@@ -202,10 +202,8 @@ static int __init dc_rtc_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, rtc);
rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
&dc_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc->rtc_dev))
- return PTR_ERR(rtc->rtc_dev);
- return 0;
+ return PTR_ERR_OR_ZERO(rtc->rtc_dev);
}
static const struct of_device_id dc_dt_ids[] = {
--
1.7.12.4
[PATCH 2/2] rtc:rtc-ds1347: Use PTR_ERR_OR_ZERO to replace the open code
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So
just replace them rather than duplicating its implement.
Signed-off-by: zhong jiang
---
drivers/rtc/rtc-ds1347.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/rtc/rtc-ds1347.c b/drivers/rtc/rtc-ds1347.c
index 938512c..b97b67f 100644
--- a/drivers/rtc/rtc-ds1347.c
+++ b/drivers/rtc/rtc-ds1347.c
@@ -155,10 +155,7 @@ static int ds1347_probe(struct spi_device *spi)
rtc = devm_rtc_device_register(&spi->dev, "ds1347",
&ds1347_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc))
- return PTR_ERR(rtc);
-
- return 0;
+ return PTR_ERR_OR_ZERO(rtc);
}
static struct spi_driver ds1347_driver = {
--
1.7.12.4
[PATCH] scsi: Use vmemdup_user to replace the open code
vmemdup_user is better than duplicating its implementation, So just
replace the open code.
Signed-off-by: zhong jiang
---
drivers/scsi/hpsa.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 58bb70b..948576a 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6381,13 +6381,9 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h,
void __user *argp)
return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
- if (!ioc) {
- status = -ENOMEM;
- goto cleanup1;
- }
- if (copy_from_user(ioc, argp, sizeof(*ioc))) {
- status = -EFAULT;
+ ioc = vmemdup_user(argp, sizeof(*ioc));
+ if (IS_ERR(ioc)) {
+ status = PTR_ERR(ioc);
goto cleanup1;
}
if ((ioc->buf_size < 1) &&
--
1.7.12.4
Re: [PATCH] scsi: Use vmemdup_user to replace the open code
Please ignore the patch, will repost . Thanks
On 2018/8/13 20:22, zhong jiang wrote:
> vmemdup_user is better than duplicating its implementation, So just
> replace the open code.
>
> Signed-off-by: zhong jiang
> ---
> drivers/scsi/hpsa.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 58bb70b..948576a 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -6381,13 +6381,9 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info
> *h, void __user *argp)
> return -EINVAL;
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> - ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
> - if (!ioc) {
> - status = -ENOMEM;
> - goto cleanup1;
> - }
> - if (copy_from_user(ioc, argp, sizeof(*ioc))) {
> - status = -EFAULT;
> + ioc = vmemdup_user(argp, sizeof(*ioc));
> + if (IS_ERR(ioc)) {
> + status = PTR_ERR(ioc);
> goto cleanup1;
> }
> if ((ioc->buf_size < 1) &&
[PATCH v2] scsi: Use vmemdup_user to replace the open code
vmemdup_user is better than duplicating its implementation, So just
replace the open code.
The issue is detected with the help of Coccinelle.
Signed-off-by: zhong jiang
---
drivers/scsi/hpsa.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 58bb70b..666ba09e5 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6381,13 +6381,9 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h,
void __user *argp)
return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
- if (!ioc) {
- status = -ENOMEM;
- goto cleanup1;
- }
- if (copy_from_user(ioc, argp, sizeof(*ioc))) {
- status = -EFAULT;
+ ioc = vmemdup_user(argp, sizeof(*ioc));
+ if (IS_ERR(ioc)) {
+ status = PTR_ERR(ioc);
goto cleanup1;
}
if ((ioc->buf_size < 1) &&
@@ -6505,7 +6501,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h,
void __user *argp)
kfree(buff);
}
kfree(buff_size);
- kfree(ioc);
+ kvfree(ioc);
return status;
}
--
1.7.12.4
Re: [PATCH] arm:pm: Use kmemdup to replace duplicating its implementation
On 2018/8/13 20:28, Vladimir Zapolskiy wrote:
> Hi zhong jiang,
>
> On 08/10/2018 05:40 AM, zhong jiang wrote:
>> kmemdup is better than kmalloc() + memcpy(), and we do not like
>> open code. So just use kmemdup instead.
>>
>> Signed-off-by: zhong jiang
>> ---
>> arch/arm/mach-lpc32xx/pm.c | 7 ++-
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-lpc32xx/pm.c b/arch/arm/mach-lpc32xx/pm.c
>> index 6247157..6eefff8 100644
>> --- a/arch/arm/mach-lpc32xx/pm.c
>> +++ b/arch/arm/mach-lpc32xx/pm.c
>> @@ -86,7 +86,8 @@ static int lpc32xx_pm_enter(suspend_state_t state)
>> void *iram_swap_area;
>>
>> /* Allocate some space for temporary IRAM storage */
>> -iram_swap_area = kmalloc(lpc32xx_sys_suspend_sz, GFP_KERNEL);
>> +iram_swap_area = kmemdup((void *)TEMP_IRAM_AREA,
>> +lpc32xx_sys_suspend_sz, GFP_KERNEL);
> there is a minor coding style issue, which is reported by checkpatch:
>
> CHECK: Alignment should match open parenthesis
> #53: FILE: arch/arm/mach-lpc32xx/pm.c:90:
>
> Other than that the change is valid, I'll fix the indentation issue
> myself and apply the change for v4.19.
Thank you for doing this. I will notice that for next time.
Sincerely,
zhong jiang
> --
> Best wishes,
> Vladimir
>
>> if (!iram_swap_area) {
>> printk(KERN_ERR
>> "PM Suspend: cannot allocate memory to save portion "
>> @@ -94,10 +95,6 @@ static int lpc32xx_pm_enter(suspend_state_t state)
>> return -ENOMEM;
>> }
>>
>> -/* Backup a small area of IRAM used for the suspend code */
>> -memcpy(iram_swap_area, (void *) TEMP_IRAM_AREA,
>> -lpc32xx_sys_suspend_sz);
>> -
>> /*
>> * Copy code to suspend system into IRAM. The suspend code
>> * needs to run from IRAM as DRAM may no longer be available
>>
>
> .
>
[PATCH] arm/mach-at91/pm: Do not double put the device node
Device node iterators put the previous value of the index variable,
so an explicit put causes a double put.
I detect the issue with the help of Coccinelle.
Signed-off-by: zhong jiang
---
arch/arm/mach-at91/pm.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 32fae4d..a5ec35f 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -143,15 +143,12 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool
set)
/* Check if enabled on SHDWC. */
if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
- goto put_node;
+ continue;
mode |= wsi->pmc_fsmr_bit;
if (wsi->set_polarity)
polarity |= wsi->pmc_fsmr_bit;
}
-
-put_node:
- of_node_put(np);
}
if (mode) {
--
1.7.12.4
Re: [PATCH] drivers/thermal/tegra: fix a doule free devce node
On 2018/8/14 14:39, Daniel Lezcano wrote:
> On 09/08/2018 15:40, zhong jiang wrote:
>> Device node iterators will get the return node. Meawhile, It is
>> also put the previous device node. An explicit put will cause
>> a double put.
> What about:
>
> Subject: drivers/thermal/tegra: Fix a double free on the device node
>
> "The function 'for_each_child_of_node' iterates over the node list by
> dropping the of_node reference of the previous node.
>
> Calling of_node_put() on the iterator is pointless and leads to an
> inconsistent refcounting in addition to a double free. Remove it."
Thank you for doing this. I am sorry for stupid issue.
Sincerely,
zhong jiang
>> Signed-off-by: zhong jiang
>> ---
>> drivers/thermal/tegra/soctherm.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/thermal/tegra/soctherm.c
>> b/drivers/thermal/tegra/soctherm.c
>> index ed28110..3aa55c9 100644
>> --- a/drivers/thermal/tegra/soctherm.c
>> +++ b/drivers/thermal/tegra/soctherm.c
>> @@ -980,7 +980,6 @@ static void soctherm_init_hw_throt_cdev(struct
>> platform_device *pdev)
>> tcd = thermal_of_cooling_device_register(np_stcc,
>> (char *)name, ts,
>> &throt_cooling_ops);
>> -of_node_put(np_stcc);
>> if (IS_ERR_OR_NULL(tcd)) {
>> dev_err(dev,
>> "throttle-cfg: %s: failed to register cooling
>> device\n",
>>
>
[PATCH v2] ia64: Use ARRAY_SIZE to replace its implementation
We prefer to ARRAY_SIZE rather than duplicating its implementation.
And just one place use the #define variable, therefore, remove
PFM_CMD_COUNT definition altogether.
Signed-off-by: zhong jiang
---
v1->v2:
- According to Joe's suggestion. remove the #define variable,
and use ARRAY_SIZE to replace directly.
arch/ia64/kernel/perfmon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index a9d4dc6..bfe0094c 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -4645,7 +4645,6 @@ static char *pfmfs_dname(struct dentry *dentry, char
*buffer, int buflen)
/* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY,
pfarg_dbreg_t, NULL),
/* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY,
pfarg_dbreg_t, NULL)
};
-#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
static int
pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
@@ -4770,7 +4769,7 @@ static char *pfmfs_dname(struct dentry *dentry, char
*buffer, int buflen)
*/
if (unlikely(pmu_conf == NULL)) return -ENOSYS;
- if (unlikely(cmd < 0 || cmd >= PFM_CMD_COUNT)) {
+ if (unlikely(cmd < 0 || cmd >= ARRAY_SIZE(pfm_cmd_tab))) {
DPRINT(("invalid cmd=%d\n", cmd));
return -EINVAL;
}
--
1.7.12.4
Re: [PATCHv2 0/2] phy: Use PTR_ERR_OR_ZERO to replace the open coded version
Ping , Anyone has any objection with the patchset ? Thanks, zhong jiang On 2018/8/16 23:58, zhong jiang wrote: > The issue is detected with the help of Coccinelle. > > v1->v2: > - According to Florian's suggestion, change the subject of the patch. > > zhong jiang (2): > phy:phy-brcm-usb: Use PTR_ERR_OR_ZERO to replace the open coded > version > phy:phy-lantiq-rcu-usb2: Use PTR_ERR_OR_ZERO to replace the open > coded version > > drivers/phy/broadcom/phy-brcm-usb.c | 4 +--- > drivers/phy/lantiq/phy-lantiq-rcu-usb2.c | 4 +--- > 2 files changed, 2 insertions(+), 6 deletions(-) >
Re: [PATCH v2] scsi: Use vmemdup_user to replace the open code
On 2018/8/13 22:20, don.br...@microchip.com wrote:
>> -Original Message-
>> From: zhong jiang [mailto:zhongji...@huawei.com]
>> Sent: Monday, August 13, 2018 7:43 AM
>> To: don.br...@microsemi.com; j...@linux.vnet.ibm.com;
>> martin.peter...@oracle.com
>> Cc: linux-kernel@vger.kernel.org
>> Subject: [PATCH v2] scsi: Use vmemdup_user to replace the open code
>>
>> EXTERNAL EMAIL
>>
>>
>> EXTERNAL EMAIL
>>
>>
>> vmemdup_user is better than duplicating its implementation, So just
>> replace the open code.
>>
>> The issue is detected with the help of Coccinelle.
>>
>> Signed-off-by: zhong jiang
> Tested-by: Don Brace
> Acked-by: Don Brace
Thanks Don for your test and ack.
Martin , Can you pick up the patch, Thanks
Best wishes,
zhong jiang
>> ---
>> drivers/scsi/hpsa.c | 12
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>> index 58bb70b..666ba09e5 100644
>> --- a/drivers/scsi/hpsa.c
>> +++ b/drivers/scsi/hpsa.c
>> @@ -6381,13 +6381,9 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info
>> *h, void __user *argp)
>> return -EINVAL;
>> if (!capable(CAP_SYS_RAWIO))
>> return -EPERM;
>> - ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
>> - if (!ioc) {
>> - status = -ENOMEM;
>> - goto cleanup1;
>> - }
>> - if (copy_from_user(ioc, argp, sizeof(*ioc))) {
>> - status = -EFAULT;
>> + ioc = vmemdup_user(argp, sizeof(*ioc));
>> + if (IS_ERR(ioc)) {
>> + status = PTR_ERR(ioc);
>> goto cleanup1;
>> }
>> if ((ioc->buf_size < 1) &&
>> @@ -6505,7 +6501,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h,
>> void __user *argp)
>> kfree(buff);
>> }
>> kfree(buff_size);
>> - kfree(ioc);
>> + kvfree(ioc);
>> return status;
>> }
>>
>> --
>> 1.7.12.4
>
>
Re: [PATCH] arm: pm: call put_device instead of of_node_put in at91_pm_config_ws
Hi, alexandre
Can you pick up the patch? Thanks
Best wishes,
zhong jiang
On 2018/8/16 18:26, zhong jiang wrote:
> of_find_device_by_node takes a reference to the struct device when it
> finds a match via get_device. but it fails to put_device in
> at91_pm_config_ws, for_each_matching_node_and_match will get and put
> the node properly, there is no need to call the of_put_node. Therefore,
> just call put_device instead of of_node_put in at91_pm_config_ws.
>
> Fixes: d7484f5c6b3b ("ARM: at91: pm: configure wakeup sources for ULP1 mode")
> Suggested-by: Claudiu Beznea
> Signed-off-by: zhong jiang
> ---
> arch/arm/mach-at91/pm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 32fae4d..6574f00 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -143,15 +143,15 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool
> set)
>
> /* Check if enabled on SHDWC. */
> if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
> - goto put_node;
> + goto put_device;
>
> mode |= wsi->pmc_fsmr_bit;
> if (wsi->set_polarity)
> polarity |= wsi->pmc_fsmr_bit;
> }
>
> -put_node:
> - of_node_put(np);
> +put_device:
> + put_device(&pdev->dev);
> }
>
> if (mode) {
Re: [PATCH] arm: pm: call put_device instead of of_node_put in at91_pm_config_ws
On 2018/8/23 20:19, Alexandre Belloni wrote:
> On 23/08/2018 19:44:38+0800, zhong jiang wrote:
>> Hi, alexandre
>>
>> Can you pick up the patch? Thanks
>>
> We are still in the merge window, this has to wait for v4.19-rc1.
Fine. Thank you for let me know .
>> Best wishes,
>> zhong jiang
>>
>> On 2018/8/16 18:26, zhong jiang wrote:
>>> of_find_device_by_node takes a reference to the struct device when it
>>> finds a match via get_device. but it fails to put_device in
>>> at91_pm_config_ws, for_each_matching_node_and_match will get and put
>>> the node properly, there is no need to call the of_put_node. Therefore,
>>> just call put_device instead of of_node_put in at91_pm_config_ws.
>>>
>>> Fixes: d7484f5c6b3b ("ARM: at91: pm: configure wakeup sources for ULP1
>>> mode")
>>> Suggested-by: Claudiu Beznea
>>> Signed-off-by: zhong jiang
>>> ---
>>> arch/arm/mach-at91/pm.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>>> index 32fae4d..6574f00 100644
>>> --- a/arch/arm/mach-at91/pm.c
>>> +++ b/arch/arm/mach-at91/pm.c
>>> @@ -143,15 +143,15 @@ static int at91_pm_config_ws(unsigned int pm_mode,
>>> bool set)
>>>
>>> /* Check if enabled on SHDWC. */
>>> if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
>>> - goto put_node;
>>> + goto put_device;
>>>
>>> mode |= wsi->pmc_fsmr_bit;
>>> if (wsi->set_polarity)
>>> polarity |= wsi->pmc_fsmr_bit;
>>> }
>>>
>>> -put_node:
>>> - of_node_put(np);
>>> +put_device:
>>> + put_device(&pdev->dev);
>>> }
>>>
>>> if (mode) {
>>
[PATCH] x86/boot/KASLR: Make local variable mem_limit static
Fix the following sparse warning:
arch/x86/boot/compressed/kaslr.c:102:20: warning: symbol 'mem_limit' was not
declared. Should it be static?
Signed-off-by: zhong jiang
---
arch/x86/boot/compressed/kaslr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 5aae509..d1e19f3 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -99,7 +99,7 @@ struct mem_vector {
/* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
-unsigned long long mem_limit = ULLONG_MAX;
+static unsigned long long mem_limit = ULLONG_MAX;
enum mem_avoid_index {
--
1.7.12.4
[PATCH linux-next] mm/tlb: make some function static to avoid warning
Fixes the following sparse warnings:
arch/x86/mm/tlb.c:38:6: warning: symbol 'clear_asid_other' was not declared.
Should it be static?
Signed-off-by: zhong jiang
---
arch/x86/mm/tlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 6eb1f34..5e8e478 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -35,7 +35,7 @@
* necessary invalidation by clearing out the 'ctx_id' which
* forces a TLB flush when the context is loaded.
*/
-void clear_asid_other(void)
+static void clear_asid_other(void)
{
u16 asid;
--
1.7.12.4
[PATCH linux-next] x86: should use NULL pointer to replace plain integer
Fixes the following sparse warnings: arch/x86/kernel/pci-iommu_table.c:63:37: warning: Using plain integer as NULL pointer Signed-off-by: zhong jiang --- arch/x86/kernel/pci-iommu_table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/pci-iommu_table.c b/arch/x86/kernel/pci-iommu_table.c index 4dfd90a..2e9006c 100644 --- a/arch/x86/kernel/pci-iommu_table.c +++ b/arch/x86/kernel/pci-iommu_table.c @@ -60,7 +60,7 @@ void __init check_iommu_entries(struct iommu_table_entry *start, printk(KERN_ERR "CYCLIC DEPENDENCY FOUND! %pS depends on %pS and vice-versa. BREAKING IT.\n", p->detect, q->detect); /* Heavy handed way..*/ - x->depend = 0; + x->depend = NULL; } } -- 1.7.12.4
[PATCH linux-next] kernel/exit: fix mismatch in function argument types
Fix following warning: kernel/exit.c:1634:6: error: symbol 'kernel_wait4' redeclared with different type (originally declared at ./include/linux/sched/task.h:78) - incompatible argument 2 (different address spaces) Signed-off-by: zhong jiang --- include/linux/sched/task.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h index 5be31eb..108ede9 100644 --- a/include/linux/sched/task.h +++ b/include/linux/sched/task.h @@ -75,7 +75,7 @@ static inline void exit_thread(struct task_struct *tsk) extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *); struct task_struct *fork_idle(int); extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags); -extern long kernel_wait4(pid_t, int *, int, struct rusage *); +extern long kernel_wait4(pid_t, int __user *, int, struct rusage *); extern void free_task(struct task_struct *tsk); -- 1.7.12.4
[PATCH linux-next] driver/gpu: Fix mismatch in funciton argument type
Fix the following warning:
drivers/gpu/drm/nouveau/dispnv50/wndw.c:570:1: error: symbol 'nv50_wndw_new_'
redeclared with different type
(originally declared at drivers/gpu/drm/nouveau/dispnv50/wndw.h:39) -
incompatible argument 7 (different signedness)
Signed-off-by: zhong jiang
---
drivers/gpu/drm/nouveau/dispnv50/wndw.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
index b0b6428..0770683 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
@@ -38,8 +38,8 @@ struct nv50_wndw {
int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *,
enum drm_plane_type, const char *name, int index,
- const u32 *format, enum nv50_disp_interlock_type,
- u32 interlock_data, u32 heads, struct nv50_wndw **);
+ const u32 *format, u32 heads, enum nv50_disp_interlock_type,
+ u32 interlock_data, struct nv50_wndw **);
void nv50_wndw_init(struct nv50_wndw *);
void nv50_wndw_fini(struct nv50_wndw *);
void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock,
--
1.7.12.4
[PATCH] driver/hwtracing: use ERR_CAST instead of ERR_PTR
Use ERR_CAT inlined function to replace the ERR_PTR(PTR_ERR). It
make the code more concise.
Signed-off-by: zhong jiang
---
drivers/hwtracing/coresight/coresight-tmc-etr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 2eda5de..1196364 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -536,7 +536,7 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table
*etr_table)
sg_table = tmc_alloc_sg_table(dev, node, nr_tpages, nr_dpages, pages);
if (IS_ERR(sg_table)) {
kfree(etr_table);
- return ERR_PTR(PTR_ERR(sg_table));
+ return ERR_CAST(sg_table);
}
etr_table->sg_table = sg_table;
--
1.7.12.4
[PATCH] lib/sg_pool,debugobjects: remove unnecessary null check when free the object
kmem_cache_destroy/mempool_destroy has taken null check into account.
so remove the redundant check.
Signed-off-by: zhong jiang
---
lib/debugobjects.c | 3 +--
lib/sg_pool.c | 7 +++
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 994be48..7a6d80b 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -1185,8 +1185,7 @@ void __init debug_objects_mem_init(void)
if (!obj_cache || debug_objects_replace_static_objects()) {
debug_objects_enabled = 0;
- if (obj_cache)
- kmem_cache_destroy(obj_cache);
+ kmem_cache_destroy(obj_cache);
pr_warn("out of memory.\n");
} else
debug_objects_selftest();
diff --git a/lib/sg_pool.c b/lib/sg_pool.c
index 6dd3061..d1c1e63 100644
--- a/lib/sg_pool.c
+++ b/lib/sg_pool.c
@@ -148,10 +148,9 @@ static __init int sg_pool_init(void)
cleanup_sdb:
for (i = 0; i < SG_MEMPOOL_NR; i++) {
struct sg_pool *sgp = sg_pools + i;
- if (sgp->pool)
- mempool_destroy(sgp->pool);
- if (sgp->slab)
- kmem_cache_destroy(sgp->slab);
+
+ mempool_destroy(sgp->pool);
+ kmem_cache_destroy(sgp->slab);
}
return -ENOMEM;
--
1.7.12.4
[PATCH] x86/platform/olpc: Use the PTR_ERR_OR_ZERO to simplify the code
use PTR_ERR_OR_ZERO is better than the open code
Signed-off-by: zhong jiang
---
arch/x86/platform/olpc/olpc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/platform/olpc/olpc.c b/arch/x86/platform/olpc/olpc.c
index 7c3077e..f0e920f 100644
--- a/arch/x86/platform/olpc/olpc.c
+++ b/arch/x86/platform/olpc/olpc.c
@@ -311,10 +311,8 @@ static int __init add_xo1_platform_devices(void)
return PTR_ERR(pdev);
pdev = platform_device_register_simple("olpc-xo1", -1, NULL, 0);
- if (IS_ERR(pdev))
- return PTR_ERR(pdev);
- return 0;
+ return PTR_ERR_OR_ZERO(pdev);
}
static int olpc_xo1_ec_probe(struct platform_device *pdev)
--
1.7.12.4
Re: [PATCH] lib/sg_pool,debugobjects: remove unnecessary null check when free the object
On 2018/8/1 0:21, Thomas Gleixner wrote: > On Tue, 31 Jul 2018, zhong jiang wrote: > >> kmem_cache_destroy/mempool_destroy has taken null check into account. >> so remove the redundant check. > Please split the patch so they can be applied independently by the relevant > maintainers. > > Thanks, > > tglx > > . > Ok, will do . Thanks, zhong jiang
[PATCH] lib/sg_pool: remove unnecessary null check when free the object
kmem_cache_destroy/mempool_destroy has taken null check into account.
so remove the redundant check.
Signed-off-by: zhong jiang
---
lib/sg_pool.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/sg_pool.c b/lib/sg_pool.c
index 6dd3061..d1c1e63 100644
--- a/lib/sg_pool.c
+++ b/lib/sg_pool.c
@@ -148,10 +148,9 @@ static __init int sg_pool_init(void)
cleanup_sdb:
for (i = 0; i < SG_MEMPOOL_NR; i++) {
struct sg_pool *sgp = sg_pools + i;
- if (sgp->pool)
- mempool_destroy(sgp->pool);
- if (sgp->slab)
- kmem_cache_destroy(sgp->slab);
+
+ mempool_destroy(sgp->pool);
+ kmem_cache_destroy(sgp->slab);
}
return -ENOMEM;
--
1.7.12.4
[PATCH] lib/debugobjects: remove redundant check when free the object
kmem_cache_destroy has taken the null check into account. so just
remove the unnecessary check.
Signed-off-by: zhong jiang
---
lib/debugobjects.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 994be48..7a6d80b 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -1185,8 +1185,7 @@ void __init debug_objects_mem_init(void)
if (!obj_cache || debug_objects_replace_static_objects()) {
debug_objects_enabled = 0;
- if (obj_cache)
- kmem_cache_destroy(obj_cache);
+ kmem_cache_destroy(obj_cache);
pr_warn("out of memory.\n");
} else
debug_objects_selftest();
--
1.7.12.4
[PATCH] kernel/module: Use kmemdup to replace kmalloc+memcpy
we prefer to the kmemdup rather than kmalloc+memcpy. so just
replace them.
Signed-off-by: zhong jiang
---
kernel/module.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 20344e4..6746c85 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2057,21 +2057,19 @@ static int copy_module_elf(struct module *mod, struct
load_info *info)
/* Elf section header table */
size = sizeof(*info->sechdrs) * info->hdr->e_shnum;
- mod->klp_info->sechdrs = kmalloc(size, GFP_KERNEL);
+ mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL);
if (mod->klp_info->sechdrs == NULL) {
ret = -ENOMEM;
goto free_info;
}
- memcpy(mod->klp_info->sechdrs, info->sechdrs, size);
/* Elf section name string table */
size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
- mod->klp_info->secstrings = kmalloc(size, GFP_KERNEL);
+ mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL);
if (mod->klp_info->secstrings == NULL) {
ret = -ENOMEM;
goto free_sechdrs;
}
- memcpy(mod->klp_info->secstrings, info->secstrings, size);
/* Elf symbol section index */
symndx = info->index.sym;
--
1.7.12.4
[PATCH] drivers/staging/mt7621-eth: Use dma_zalloc_coherent to replace dma_alloc_coherent+memset
we prefer to use dma_zalloc_coherent rather than dam_alloc_coherent+memset
Signed-off-by: zhong jiang
---
drivers/staging/mt7621-eth/mtk_eth_soc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c
b/drivers/staging/mt7621-eth/mtk_eth_soc.c
index f9b593ca..7135075 100644
--- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
+++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
@@ -1396,14 +1396,13 @@ static int mtk_qdma_tx_alloc_tx(struct mtk_eth *eth)
if (!ring->tx_buf)
goto no_tx_mem;
- ring->tx_dma = dma_alloc_coherent(eth->dev,
+ ring->tx_dma = dma_zalloc_coherent(eth->dev,
ring->tx_ring_size * sz,
&ring->tx_phys,
GFP_ATOMIC | __GFP_ZERO);
if (!ring->tx_dma)
goto no_tx_mem;
- memset(ring->tx_dma, 0, ring->tx_ring_size * sz);
for (i = 0; i < ring->tx_ring_size; i++) {
int next = (i + 1) % ring->tx_ring_size;
u32 next_ptr = ring->tx_phys + next * sz;
--
1.7.12.4
[Question] A novel case happened when using mempool allocate memory.
Hi, Everyone
I ran across the following novel case similar to memory leak in linux-4.1
stable when allocating
memory object by kmem_cache_alloc. it rarely can be reproduced.
I create a specific mempool with 24k size based on the slab. it can not be
merged with
other kmem cache. I record the allocation and free usage by atomic_add/sub.
After a while,
I watch the specific slab consume most of total memory. After halting the
code execution.
The counter of allocation and free is equal. Therefore, I am sure that module
have released
all meory resource. but the statistic of specific slab is very high but stable
by checking /proc/slabinfo.
but It is strange that the specific slab will free get back all memory when
unregister the module.
I got the following information from specific slab data structure when halt the
module execution.
kmem_cache_node
kmem_cache
nr_partial = 1,
min_partial = 7
partial = {
cpu_partial = 2
next = 0x7c00085cae20 object_size =
24576
prev = 0x7c00085cae20
},
nr_slabs = {
counter = 365610
},
total_objects = {
counter = 365610
},
full = {
next = 0x8013e44f75f0,
prev = 0x8013e44f75f0
},
>From the above restricted information , we can know that the node full list is
>empty. and partial list only
have a slab. A slab contain a object. I think that most of slab stay in the
cpu_partial
list even though it seems to be impossible theoretically. because I come to
the conclusion based on the case
that slab take up the memory will be release when unregister the moudle.
but I check the code(mm/slub.c) carefully . I can not find any clue to prove my
assumption.
I will be appreciate if anyone have any idea about the case.
Thanks
zhong jiang
[PATCH] fs/binfmt_elf: remove the same condition check
dump_align is used to double check in a expression. It is redundant. so just remove one of them. Signed-off-by: zhong jiang --- fs/binfmt_elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index efae2fb..b6c5b02 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1439,7 +1439,7 @@ static int writenote(struct memelfnote *men, struct coredump_params *cprm) return dump_emit(cprm, &en, sizeof(en)) && dump_emit(cprm, men->name, en.n_namesz) && dump_align(cprm, 4) && - dump_emit(cprm, men->data, men->datasz) && dump_align(cprm, 4); + dump_emit(cprm, men->data, men->datasz); } static void fill_elf_header(struct elfhdr *elf, int segs, -- 1.7.12.4
[PATCH] fs/binfmt_elf_fdpic: remove redundant condition check in writenote
It is unncessary to use double test for a expression. so just remove one of them. Signed-off-by: zhong jiang --- fs/binfmt_elf_fdpic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index b53bb37..5162372 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -1288,7 +1288,7 @@ static int writenote(struct memelfnote *men, struct coredump_params *cprm) return dump_emit(cprm, &en, sizeof(en)) && dump_emit(cprm, men->name, en.n_namesz) && dump_align(cprm, 4) && - dump_emit(cprm, men->data, men->datasz) && dump_align(cprm, 4); + dump_emit(cprm, men->data, men->datasz); } static inline void fill_elf_fdpic_header(struct elfhdr *elf, int segs) -- 1.7.12.4
Re: [PATCH] fs/binfmt_elf_fdpic: remove redundant condition check in writenote
plese ingore it. On 2018/8/2 10:04, zhong jiang wrote: > It is unncessary to use double test for a expression. so just > remove one of them. > > Signed-off-by: zhong jiang > --- > fs/binfmt_elf_fdpic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c > index b53bb37..5162372 100644 > --- a/fs/binfmt_elf_fdpic.c > +++ b/fs/binfmt_elf_fdpic.c > @@ -1288,7 +1288,7 @@ static int writenote(struct memelfnote *men, struct > coredump_params *cprm) > > return dump_emit(cprm, &en, sizeof(en)) && > dump_emit(cprm, men->name, en.n_namesz) && dump_align(cprm, 4) > && > - dump_emit(cprm, men->data, men->datasz) && dump_align(cprm, 4); > + dump_emit(cprm, men->data, men->datasz); > } > > static inline void fill_elf_fdpic_header(struct elfhdr *elf, int segs)
Re: [PATCH] fs/binfmt_elf: remove the same condition check
On 2018/8/2 10:34, Matthew Wilcox wrote: > On Thu, Aug 02, 2018 at 10:00:28AM +0800, zhong jiang wrote: >> dump_align is used to double check in a expression. It is redundant. >> so just remove one of them. > You're wrong. Functions in C can have side-effects (and this one does). > > Ok, I miss that. Thank you for clarification. Thanks, zhong jiang
Re: [PATCH] fs/binfmt_elf: remove the same condition check
On 2018/8/2 11:04, Al Viro wrote: > On Thu, Aug 02, 2018 at 10:00:28AM +0800, zhong jiang wrote: >> dump_align is used to double check in a expression. It is redundant. >> so just remove one of them. > > What makes you think that it is redundant? > I am sorry for that. Maybe I miss something. Thanks zhong jiang >> Signed-off-by: zhong jiang >> --- >> fs/binfmt_elf.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c >> index efae2fb..b6c5b02 100644 >> --- a/fs/binfmt_elf.c >> +++ b/fs/binfmt_elf.c >> @@ -1439,7 +1439,7 @@ static int writenote(struct memelfnote *men, struct >> coredump_params *cprm) >> >> return dump_emit(cprm, &en, sizeof(en)) && >> dump_emit(cprm, men->name, en.n_namesz) && dump_align(cprm, 4) && >> -dump_emit(cprm, men->data, men->datasz) && dump_align(cprm, 4); >> +dump_emit(cprm, men->data, men->datasz); >> } >> >> static void fill_elf_header(struct elfhdr *elf, int segs, >> -- >> 1.7.12.4 >> > . >
Re: [Question] A novel case happened when using mempool allocate memory.
On 2018/8/1 23:37, Matthew Wilcox wrote:
> On Wed, Aug 01, 2018 at 11:31:15PM +0800, zhong jiang wrote:
>> Hi, Everyone
>>
>> I ran across the following novel case similar to memory leak in linux-4.1
>> stable when allocating
>> memory object by kmem_cache_alloc. it rarely can be reproduced.
>>
>> I create a specific mempool with 24k size based on the slab. it can not be
>> merged with
>> other kmem cache. I record the allocation and free usage by
>> atomic_add/sub.After a while,
>> I watch the specific slab consume most of total memory. After halting the
>> code execution.
>> The counter of allocation and free is equal. Therefore, I am sure that
>> module have released
>> all meory resource. but the statistic of specific slab is very high but
>> stable by checking /proc/slabinfo.
> Please post the code.
>
> .
>
when module is loaded. we create the specific mempool. The code flow is as
follows.
mem_pool_create() {
slab_cache = kmem_cache_create(name, item_size, 0, 0 , NULL);
mempoll_create(min_pool_size, mempool_alloc_slab, mempool_free_slab,
slab_cache); //min_pool_size is assigned to 1024
atomic_set(pool->statistics, 0);
}
we allocate memory from specific mempool , The code flow is as follows.
mem_alloc() {
mempool_alloc(pool, gfp_flags);
atomic_inc(pool->statistics);
}
we release memory to specific mempool . The code flow is as follows.
mem_free() {
mempool_free(object_ptr, pool);
atomic_dec(pool->statistics);
}
when we unregister the module, the memory has been taken up will get back the
system.
the code flow is as follows.
mem_pool_destroy() {
mempool_destroy(pool);
kmem_cache_destroy(slab_cache);
}
>From the above information. I assume the specific kmem_cache will not take up
>overmuch memory
when halting the execution and pool->statistics is equal to 0.
I have no idea about the issue.
Thanks
zhong jiang
[PATCH v2 3/4] drivers/scsi/snic/snic_trc: Use kvcalloc instead of vmalloc+memset
The kvcalloc is better than vmalloc() + memset() , So replace them.
Signed-off-by: zhong jiang
---
drivers/scsi/snic/snic_trc.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/snic/snic_trc.c b/drivers/scsi/snic/snic_trc.c
index fc60c93..e766fa2 100644
--- a/drivers/scsi/snic/snic_trc.c
+++ b/drivers/scsi/snic/snic_trc.c
@@ -125,8 +125,8 @@ struct snic_trc_data *
void *tbuf = NULL;
int tbuf_sz = 0, ret;
- tbuf_sz = (snic_trace_max_pages * PAGE_SIZE);
- tbuf = vmalloc(tbuf_sz);
+ tbuf_sz = snic_trace_max_pages * PAGE_SIZE;
+ tbuf = kvcalloc(snic_trace_max_pages, PAGE_SIZE, GFP_KERNEL);
if (!tbuf) {
SNIC_ERR("Failed to Allocate Trace Buffer Size. %d\n", tbuf_sz);
SNIC_ERR("Trace Facility not enabled.\n");
@@ -135,7 +135,6 @@ struct snic_trc_data *
return ret;
}
- memset(tbuf, 0, tbuf_sz);
trc->buf = (struct snic_trc_data *) tbuf;
spin_lock_init(&trc->lock);
@@ -173,7 +172,7 @@ struct snic_trc_data *
snic_trc_debugfs_term();
if (trc->buf) {
- vfree(trc->buf);
+ kvfree(trc->buf);
trc->buf = NULL;
}
--
1.7.12.4
[PATCH v2 0/4] Use dma_zalloc_coherent and kvcalloc to replace open code
v1->v2: According to Andy and Joe suggestions. [patch 1/4] get rid of unnessary parens and use kvcalloc and kvfree to replace the vmalloc and vfree. [patch 2/4] modify the changelog as Andy's suggestion. [patch 3/4] replace vmalloc and vfree by using kvcalloc adn kvfree. [patch 4/4] do nothing. just add Andy's review-by. zhong jiang (4): driver/scsi/fnic/fnic_trace: Use kvcalloc instead of vmalloc+memset drivers/scsi/dpt_i2o: Use dma_zalloc_coherent to repalce dma_alloc_coherent+memset drivers/scsi/snic/snic_trc: Use kvcalloc instead of vmalloc+memset drivers/scsi/mvsas/mv_init: Use dma_zalloc_coherent to replace dma_alloc_coherent+memset drivers/scsi/dpt_i2o.c | 12 drivers/scsi/fnic/fnic_trace.c | 7 +++ drivers/scsi/mvsas/mv_init.c | 12 drivers/scsi/snic/snic_trc.c | 7 +++ 4 files changed, 14 insertions(+), 24 deletions(-) -- 1.7.12.4
[PATCH linux-next] gpio: fix meaningless return expression
Fix the following sparse error:
drivers/gpio/gpio-ath79.c:54:16: error: return expression in void function
Signed-off-by: zhong jiang
---
drivers/gpio/gpio-ath79.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-ath79.c b/drivers/gpio/gpio-ath79.c
index 684e9d6..0a553d6 100644
--- a/drivers/gpio/gpio-ath79.c
+++ b/drivers/gpio/gpio-ath79.c
@@ -51,7 +51,7 @@ static u32 ath79_gpio_read(struct ath79_gpio_ctrl *ctrl,
unsigned reg)
static void ath79_gpio_write(struct ath79_gpio_ctrl *ctrl,
unsigned reg, u32 val)
{
- return writel(val, ctrl->base + reg);
+ writel(val, ctrl->base + reg);
}
static bool ath79_gpio_update_bits(
--
1.7.12.4
Re: [PATCH v11 19/26] mm: provide speculative fault infrastructure
On 2018/7/25 0:10, Laurent Dufour wrote:
>
> On 24/07/2018 16:26, zhong jiang wrote:
>> On 2018/5/17 19:06, Laurent Dufour wrote:
>>> From: Peter Zijlstra
>>>
>>> Provide infrastructure to do a speculative fault (not holding
>>> mmap_sem).
>>>
>>> The not holding of mmap_sem means we can race against VMA
>>> change/removal and page-table destruction. We use the SRCU VMA freeing
>>> to keep the VMA around. We use the VMA seqcount to detect change
>>> (including umapping / page-table deletion) and we use gup_fast() style
>>> page-table walking to deal with page-table races.
>>>
>>> Once we've obtained the page and are ready to update the PTE, we
>>> validate if the state we started the fault with is still valid, if
>>> not, we'll fail the fault with VM_FAULT_RETRY, otherwise we update the
>>> PTE and we're done.
>>>
>>> Signed-off-by: Peter Zijlstra (Intel)
>>>
>>> [Manage the newly introduced pte_spinlock() for speculative page
>>> fault to fail if the VMA is touched in our back]
>>> [Rename vma_is_dead() to vma_has_changed() and declare it here]
>>> [Fetch p4d and pud]
>>> [Set vmd.sequence in __handle_mm_fault()]
>>> [Abort speculative path when handle_userfault() has to be called]
>>> [Add additional VMA's flags checks in handle_speculative_fault()]
>>> [Clear FAULT_FLAG_ALLOW_RETRY in handle_speculative_fault()]
>>> [Don't set vmf->pte and vmf->ptl if pte_map_lock() failed]
>>> [Remove warning comment about waiting for !seq&1 since we don't want
>>> to wait]
>>> [Remove warning about no huge page support, mention it explictly]
>>> [Don't call do_fault() in the speculative path as __do_fault() calls
>>> vma->vm_ops->fault() which may want to release mmap_sem]
>>> [Only vm_fault pointer argument for vma_has_changed()]
>>> [Fix check against huge page, calling pmd_trans_huge()]
>>> [Use READ_ONCE() when reading VMA's fields in the speculative path]
>>> [Explicitly check for __HAVE_ARCH_PTE_SPECIAL as we can't support for
>>> processing done in vm_normal_page()]
>>> [Check that vma->anon_vma is already set when starting the speculative
>>> path]
>>> [Check for memory policy as we can't support MPOL_INTERLEAVE case due to
>>> the processing done in mpol_misplaced()]
>>> [Don't support VMA growing up or down]
>>> [Move check on vm_sequence just before calling handle_pte_fault()]
>>> [Don't build SPF services if !CONFIG_SPECULATIVE_PAGE_FAULT]
>>> [Add mem cgroup oom check]
>>> [Use READ_ONCE to access p*d entries]
>>> [Replace deprecated ACCESS_ONCE() by READ_ONCE() in vma_has_changed()]
>>> [Don't fetch pte again in handle_pte_fault() when running the speculative
>>> path]
>>> [Check PMD against concurrent collapsing operation]
>>> [Try spin lock the pte during the speculative path to avoid deadlock with
>>> other CPU's invalidating the TLB and requiring this CPU to catch the
>>> inter processor's interrupt]
>>> [Move define of FAULT_FLAG_SPECULATIVE here]
>>> [Introduce __handle_speculative_fault() and add a check against
>>> mm->mm_users in handle_speculative_fault() defined in mm.h]
>>> Signed-off-by: Laurent Dufour
>>> ---
>>> include/linux/hugetlb_inline.h | 2 +-
>>> include/linux/mm.h | 30
>>> include/linux/pagemap.h| 4 +-
>>> mm/internal.h | 16 +-
>>> mm/memory.c| 340
>>> -
>>> 5 files changed, 385 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/linux/hugetlb_inline.h b/include/linux/hugetlb_inline.h
>>> index 0660a03d37d9..9e25283d6fc9 100644
>>> --- a/include/linux/hugetlb_inline.h
>>> +++ b/include/linux/hugetlb_inline.h
>>> @@ -8,7 +8,7 @@
>>>
>>> static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
>>> {
>>> - return !!(vma->vm_flags & VM_HUGETLB);
>>> + return !!(READ_ONCE(vma->vm_flags) & VM_HUGETLB);
>>> }
>>>
>>> #else
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index 05cbba70104b..31acf98a7d92 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -315,6 +315,7 @@ extern pgprot_t protection_map[16];
>>> #define FAULT
Re: [PATCH v11 19/26] mm: provide speculative fault infrastructure
On 2018/7/25 18:44, Laurent Dufour wrote: > > On 25/07/2018 11:04, zhong jiang wrote: >> On 2018/7/25 0:10, Laurent Dufour wrote: >>> On 24/07/2018 16:26, zhong jiang wrote: >>>> On 2018/5/17 19:06, Laurent Dufour wrote: >>>>> From: Peter Zijlstra >>>>> >>>>> Provide infrastructure to do a speculative fault (not holding >>>>> mmap_sem). >>>>> >>>>> The not holding of mmap_sem means we can race against VMA >>>>> change/removal and page-table destruction. We use the SRCU VMA freeing >>>>> to keep the VMA around. We use the VMA seqcount to detect change >>>>> (including umapping / page-table deletion) and we use gup_fast() style >>>>> page-table walking to deal with page-table races. >>>>> >>>>> Once we've obtained the page and are ready to update the PTE, we >>>>> validate if the state we started the fault with is still valid, if >>>>> not, we'll fail the fault with VM_FAULT_RETRY, otherwise we update the >>>>> PTE and we're done. >>>>> >>>>> Signed-off-by: Peter Zijlstra (Intel) >>>>> >>>>> [Manage the newly introduced pte_spinlock() for speculative page >>>>> fault to fail if the VMA is touched in our back] >>>>> [Rename vma_is_dead() to vma_has_changed() and declare it here] >>>>> [Fetch p4d and pud] >>>>> [Set vmd.sequence in __handle_mm_fault()] >>>>> [Abort speculative path when handle_userfault() has to be called] >>>>> [Add additional VMA's flags checks in handle_speculative_fault()] >>>>> [Clear FAULT_FLAG_ALLOW_RETRY in handle_speculative_fault()] >>>>> [Don't set vmf->pte and vmf->ptl if pte_map_lock() failed] >>>>> [Remove warning comment about waiting for !seq&1 since we don't want >>>>> to wait] >>>>> [Remove warning about no huge page support, mention it explictly] >>>>> [Don't call do_fault() in the speculative path as __do_fault() calls >>>>> vma->vm_ops->fault() which may want to release mmap_sem] >>>>> [Only vm_fault pointer argument for vma_has_changed()] >>>>> [Fix check against huge page, calling pmd_trans_huge()] >>>>> [Use READ_ONCE() when reading VMA's fields in the speculative path] >>>>> [Explicitly check for __HAVE_ARCH_PTE_SPECIAL as we can't support for >>>>> processing done in vm_normal_page()] >>>>> [Check that vma->anon_vma is already set when starting the speculative >>>>> path] >>>>> [Check for memory policy as we can't support MPOL_INTERLEAVE case due to >>>>> the processing done in mpol_misplaced()] >>>>> [Don't support VMA growing up or down] >>>>> [Move check on vm_sequence just before calling handle_pte_fault()] >>>>> [Don't build SPF services if !CONFIG_SPECULATIVE_PAGE_FAULT] >>>>> [Add mem cgroup oom check] >>>>> [Use READ_ONCE to access p*d entries] >>>>> [Replace deprecated ACCESS_ONCE() by READ_ONCE() in vma_has_changed()] >>>>> [Don't fetch pte again in handle_pte_fault() when running the speculative >>>>> path] >>>>> [Check PMD against concurrent collapsing operation] >>>>> [Try spin lock the pte during the speculative path to avoid deadlock with >>>>> other CPU's invalidating the TLB and requiring this CPU to catch the >>>>> inter processor's interrupt] >>>>> [Move define of FAULT_FLAG_SPECULATIVE here] >>>>> [Introduce __handle_speculative_fault() and add a check against >>>>> mm->mm_users in handle_speculative_fault() defined in mm.h] >>>>> Signed-off-by: Laurent Dufour >>>>> --- >>>>> include/linux/hugetlb_inline.h | 2 +- >>>>> include/linux/mm.h | 30 >>>>> include/linux/pagemap.h| 4 +- >>>>> mm/internal.h | 16 +- >>>>> mm/memory.c| 340 >>>>> - >>>>> 5 files changed, 385 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/include/linux/hugetlb_inline.h >>>>> b/include/linux/hugetlb_inline.h >>>>> index 0660a03d37d9..9e25283d6fc9 100644 >>>>> --- a/include/linu
[PATCH] misc: cxl: Move a deference below a NULL test
It is safe to move a deference below a NULL test.
Signed-off-by: zhong jiang
---
drivers/misc/cxl/guest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index 3bc0c15..559e835 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -1018,11 +1018,11 @@ int cxl_guest_init_afu(struct cxl *adapter, int slice,
struct device_node *afu_n
void cxl_guest_remove_afu(struct cxl_afu *afu)
{
- pr_devel("in %s - AFU(%d)\n", __func__, afu->slice);
-
if (!afu)
return;
+ pr_devel("in %s - AFU(%d)\n", __func__, afu->slice);
+
/* flush and stop pending job */
afu->guest->handle_err = false;
flush_delayed_work(&afu->guest->work_err);
--
1.7.12.4
Re: [PATCH v2] NFC: st-nci: remove a redundant null pointer check
On 2018/9/26 22:03, Andy Shevchenko wrote:
> On Wed, Sep 26, 2018 at 08:30:50PM +0800, zhong jiang wrote:
>> The dev is impossible is NULL. hence the check is redundant. We
>> never will hit it.
>>
> Reviewed-by: Andy Shevchenko
Thanks.
>> Signed-off-by: zhong jiang
>> ---
>> v1->v2:
>> - According to Greg's suggestion. just remove the null pointer will be
>> better.
>>
>> drivers/nfc/st-nci/spi.c | 7 ---
>> 1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/nfc/st-nci/spi.c b/drivers/nfc/st-nci/spi.c
>> index 1470559..864fd90 100644
>> --- a/drivers/nfc/st-nci/spi.c
>> +++ b/drivers/nfc/st-nci/spi.c
>> @@ -233,13 +233,6 @@ static int st_nci_spi_probe(struct spi_device *dev)
>> dev_dbg(&dev->dev, "%s\n", __func__);
>> dev_dbg(&dev->dev, "IRQ: %d\n", dev->irq);
>>
>> -/* Check SPI platform functionnalities */
>> -if (!dev) {
>> -pr_debug("%s: dev is NULL. Device is not accessible.\n",
>> -__func__);
>> -return -ENODEV;
>> -}
>> -
>> phy = devm_kzalloc(&dev->dev, sizeof(struct st_nci_spi_phy),
>> GFP_KERNEL);
>> if (!phy)
>> --
>> 1.7.12.4
>>
Re: [PATCH] powerpc: Move a dereference below a NULL test
On 2018/9/26 22:22, Michal Suchánek wrote: > On Wed, 26 Sep 2018 19:46:08 +0800 > zhong jiang wrote: > >> It is safe to move dereference below a NULL test. >> >> Signed-off-by: zhong jiang >> --- >> arch/powerpc/kernel/cacheinfo.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/kernel/cacheinfo.c >> b/arch/powerpc/kernel/cacheinfo.c index a8f20e5..7f19714 100644 >> --- a/arch/powerpc/kernel/cacheinfo.c >> +++ b/arch/powerpc/kernel/cacheinfo.c >> @@ -401,14 +401,13 @@ static struct cache >> *cache_lookup_or_instantiate(struct device_node *node, struct cache >> *cache; >> cache = cache_lookup_by_node(node); >> +if (!cache) >> +cache = cache_do_one_devnode(node, level); >> >> WARN_ONCE(cache && cache->level != level, > This has also null test so cache should be dereferenced only when > non-null here. :-[ , you're right. I forget WARN_ONCE. please ignore the patch. Sincerely, zhong jiang > Thanks > > Michal > > . >
Re: [PATCHv2] misc: genwqe: remove duplicated include and order header files alphabetically in card_utils.c
On 2018/9/26 11:05, zhong jiang wrote: > dma-mapping.h and delay.h have included twice. It is unnecessary. Meanwhile, > Arrange header files in alphabetical sequence to improve readability. > > Signed-off-by: zhong jiang > --- > drivers/misc/genwqe/card_utils.c | 22 ++ > 1 file changed, 10 insertions(+), 12 deletions(-) > > diff --git a/drivers/misc/genwqe/card_utils.c > b/drivers/misc/genwqe/card_utils.c > index 8679e0b..994e8fa 100644 > --- a/drivers/misc/genwqe/card_utils.c > +++ b/drivers/misc/genwqe/card_utils.c > @@ -22,26 +22,24 @@ > * Miscelanous functionality used in the other GenWQE driver parts. > */ > > -#include > +#include > +#include > +#include > #include > -#include > -#include > -#include > -#include > #include > #include > -#include > -#include > -#include > -#include > +#include > #include > +#include > +#include > #include > -#include > -#include > +#include > +#include > +#include > > -#include "genwqe_driver.h" > #include "card_base.h" > #include "card_ddcb.h" > +#include "genwqe_driver.h" > > /** > * __genwqe_writeq() - Write 64-bit register Please igore the patch. I will repost . Thanks, Sincerely, zhong jiang
[RESEND PATCH v2] misc: card_utils: remove duplicated include file
delay.h and dma-mapping.h have duplicated include. hence just remove redundant file. Signed-off-by: zhong jiang --- v1->v2: - modify the subject drivers/misc/genwqe/card_utils.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c index 8679e0b..61d647a 100644 --- a/drivers/misc/genwqe/card_utils.c +++ b/drivers/misc/genwqe/card_utils.c @@ -23,14 +23,12 @@ */ #include -#include #include #include #include #include #include #include -#include #include #include #include -- 1.7.12.4
[STABLE PATCH] slub: make ->cpu_partial unsigned int
From: Alexey Dobriyan
/*
* cpu_partial determined the maximum number of objects
* kept in the per cpu partial lists of a processor.
*/
Can't be negative.
I hit a real issue that it will result in a large number of memory leak.
Because Freeing slabs are in interrupt context. So it can trigger this issue.
put_cpu_partial can be interrupted more than once.
due to a union struct of lru and pobjects in struct page, when other core
handles
page->lru list, for eaxmple, remove_partial in freeing slab code flow, It will
result in pobjects being a negative value(0xdead). Therefore, a large number
of slabs will be added to per_cpu partial list.
I had posted the issue to community before. The detailed issue description is
as follows.
Link: https://www.spinics.net/lists/kernel/msg2870979.html
After applying the patch, The issue is fixed. So the patch is a effective
bugfix.
It should go into stable.
Signed-off-by: Alexey Dobriyan
Acked-by: Christoph Lameter
Cc: Pekka Enberg
Cc: David Rientjes
Cc: Joonsoo Kim
Cc: sta...@vger.kernel.org
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
Signed-off-by: zhong jiang
---
include/linux/slub_def.h | 3 ++-
mm/slub.c| 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 3388511..9b681f2 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -67,7 +67,8 @@ struct kmem_cache {
int size; /* The size of an object including meta data */
int object_size;/* The size of an object without meta data */
int offset; /* Free pointer offset. */
- int cpu_partial;/* Number of per cpu partial objects to keep
around */
+ /* Number of per cpu partial objects to keep around */
+ unsigned int cpu_partial;
struct kmem_cache_order_objects oo;
/* Allocation and freeing of slabs */
diff --git a/mm/slub.c b/mm/slub.c
index 2284c43..c33b0e1 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1661,7 +1661,7 @@ static void *get_partial_node(struct kmem_cache *s,
struct kmem_cache_node *n,
{
struct page *page, *page2;
void *object = NULL;
- int available = 0;
+ unsigned int available = 0;
int objects;
/*
@@ -4674,10 +4674,10 @@ static ssize_t cpu_partial_show(struct kmem_cache *s,
char *buf)
static ssize_t cpu_partial_store(struct kmem_cache *s, const char *buf,
size_t length)
{
- unsigned long objects;
+ unsigned int objects;
int err;
- err = kstrtoul(buf, 10, &objects);
+ err = kstrtouint(buf, 10, &objects);
if (err)
return err;
if (objects && !kmem_cache_has_cpu_partial(s))
--
1.7.12.4
Re: [PATCH] ieee802154: remove a redundant local variable 'i'
On 2018/9/27 22:47, Stefan Schmidt wrote:
> Hello.
>
> On 19/09/2018 16:41, zhong jiang wrote:
>> The local variable 'i' is never used after being assigned.
>> hence it should be redundant adn can be removed.
>>
>> Signed-off-by: zhong jiang
>> ---
>> net/ieee802154/nl802154.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
>> index 99f6c25..5b90151 100644
>> --- a/net/ieee802154/nl802154.c
>> +++ b/net/ieee802154/nl802154.c
>> @@ -445,7 +445,6 @@ static int nl802154_send_wpan_phy(struct
>> cfg802154_registered_device *rdev,
>> {
>> struct nlattr *nl_cmds;
>> void *hdr;
>> -int i;
>>
>> hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
>> if (!hdr)
>> @@ -508,7 +507,6 @@ static int nl802154_send_wpan_phy(struct
>> cfg802154_registered_device *rdev,
>> if (!nl_cmds)
>> goto nla_put_failure;
>>
>> -i = 0;
>> #define CMD(op, n) \
>> do {\
>> if (rdev->ops->op) {\
>>
> Sorry, but this patch is wrong. The variable i is used in line 513
> inside the CMD() macro. The compiler clearly tells this when running
> with your patch:
>
> net/ieee802154/nl802154.c: In function ‘nl802154_send_wpan_phy’:
> net/ieee802154/nl802154.c:513:4: error: ‘i’ undeclared (first use in
> this function)
>
> I would appreciate if patches sent out would at least be compile tested.
Sorry, I really double check and compile test.
Thanks,
zhong jiang
> regards
> Stefan Schmidt
>
> .
>
Re: [STABLE PATCH] slub: make ->cpu_partial unsigned int
On 2018/9/27 23:46, Greg KH wrote: > On Thu, Sep 27, 2018 at 10:43:40PM +0800, zhong jiang wrote: >> From: Alexey Dobriyan >> >> /* >> * cpu_partial determined the maximum number of objects >> * kept in the per cpu partial lists of a processor. >> */ >> >> Can't be negative. >> >> I hit a real issue that it will result in a large number of memory leak. >> Because Freeing slabs are in interrupt context. So it can trigger this issue. >> put_cpu_partial can be interrupted more than once. >> due to a union struct of lru and pobjects in struct page, when other core >> handles >> page->lru list, for eaxmple, remove_partial in freeing slab code flow, It >> will >> result in pobjects being a negative value(0xdead). Therefore, a large >> number >> of slabs will be added to per_cpu partial list. >> >> I had posted the issue to community before. The detailed issue description >> is as follows. >> >> Link: https://www.spinics.net/lists/kernel/msg2870979.html >> >> After applying the patch, The issue is fixed. So the patch is a effective >> bugfix. >> It should go into stable. > > > This is not the correct way to submit patches for inclusion in the > stable kernel tree. Please read: > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > for how to do this properly. > > Will resend with proper format. Thanks, zhong jiang
[PATCH] fs/dax.c: Use vmf_error() helper
These codes can be replaced with new inline vmf_error().
Signed-off-by: zhong jiang
---
fs/dax.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index b68ce48..e732f70 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1311,9 +1311,8 @@ static vm_fault_t dax_fault_return(int error)
{
if (error == 0)
return VM_FAULT_NOPAGE;
- if (error == -ENOMEM)
- return VM_FAULT_OOM;
- return VM_FAULT_SIGBUS;
+
+ return vmf_error(error);
}
/*
--
1.7.12.4
Re: [PATCH] mm: use this_cpu_cmpxchg_double in put_cpu_partial
On 2018/11/17 9:33, Wengang Wang wrote:
> The this_cpu_cmpxchg makes the do-while loop pass as long as the
> s->cpu_slab->partial as the same value. It doesn't care what happened to
> that slab. Interrupt is not disabled, and new alloc/free can happen in the
> interrupt handlers. Theoretically, after we have a reference to the it,
> stored in _oldpage_, the first slab on the partial list on this CPU can be
> moved to kmem_cache_node and then moved to different kmem_cache_cpu and
> then somehow can be added back as head to partial list of current
> kmem_cache_cpu, though that is a very rare case. If that rare case really
> happened, the reading of oldpage->pobjects may get a 0xdead
> unexpectedly, stored in _pobjects_, if the reading happens just after
> another CPU removed the slab from kmem_cache_node, setting lru.prev to
> LIST_POISON2 (0xdead0200). The wrong _pobjects_(negative) then
> prevents slabs from being moved to kmem_cache_node and being finally freed.
>
> We see in a vmcore, there are 375210 slabs kept in the partial list of one
> kmem_cache_cpu, but only 305 in-use objects in the same list for
> kmalloc-2048 cache. We see negative values for page.pobjects, the last page
> with negative _pobjects_ has the value of 0xdead0004, the next page looks
> good (_pobjects is 1).
>
> For the fix, I wanted to call this_cpu_cmpxchg_double with
> oldpage->pobjects, but failed due to size difference between
> oldpage->pobjects and cpu_slab->partial. So I changed to call
> this_cpu_cmpxchg_double with _tid_. I don't really want no alloc/free
> happen in between, but just want to make sure the first slab did expereince
> a remove and re-add. This patch is more to call for ideas.
Have you hit the really issue or just review the code ?
I did hit the issue and fixed in the upstream patch unpredictably by the
following patch.
e5d9998f3e09 ("slub: make ->cpu_partial unsigned int")
Thanks,
zhong jiang
> Signed-off-by: Wengang Wang
> ---
> mm/slub.c | 20 +---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index e3629cd..26539e6 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2248,6 +2248,7 @@ static void put_cpu_partial(struct kmem_cache *s,
> struct page *page, int drain)
> {
> #ifdef CONFIG_SLUB_CPU_PARTIAL
> struct page *oldpage;
> + unsigned long tid;
> int pages;
> int pobjects;
>
> @@ -2255,8 +2256,12 @@ static void put_cpu_partial(struct kmem_cache *s,
> struct page *page, int drain)
> do {
> pages = 0;
> pobjects = 0;
> - oldpage = this_cpu_read(s->cpu_slab->partial);
>
> + tid = this_cpu_read(s->cpu_slab->tid);
> + /* read tid before reading oldpage */
> + barrier();
> +
> + oldpage = this_cpu_read(s->cpu_slab->partial);
> if (oldpage) {
> pobjects = oldpage->pobjects;
> pages = oldpage->pages;
> @@ -2283,8 +2288,17 @@ static void put_cpu_partial(struct kmem_cache *s,
> struct page *page, int drain)
> page->pobjects = pobjects;
> page->next = oldpage;
>
> - } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page)
> - != oldpage);
> + /* we dont' change tid, but want to make sure it didn't change
> + * in between. We don't really hope alloc/free not happen on
> + * this CPU, but don't want the first slab be removed from and
> + * then re-added as head to this partial list. If that case
> + * happened, pobjects may read 0xdead when this slab is just
> + * removed from kmem_cache_node by other CPU setting lru.prev
> + * to LIST_POISON2.
> + */
> + } while (this_cpu_cmpxchg_double(s->cpu_slab->partial, s->cpu_slab->tid,
> + oldpage, tid, page, tid) == 0);
> +
> if (unlikely(!s->cpu_partial)) {
> unsigned long flags;
>
Re: [Question] There is a thp count left when the process exits
+LKML I can not find the possibility when I check the code. because the mmap_sem and spin_lock will protect the concurrence. I will be appreciated if anyone has some clue. Thanks, zhong jiang On 2018/11/8 23:01, zhong jiang wrote: > Hi, > > Recently, I hit the following issue in linux 3.10 stable. and hard to > recur. > > bad pmd 880c13ecea80(8017b16000e7) > > Call Trace: > [] dump_stack+0x19/0x1b > [] warn_slowpath_common+0x70/0xb0 > [] warn_slowpath_null+0x1a/0x20 > [] exit_mmap+0x196/0x1a0 > [] mmput+0x67/0xf0 > [] do_exit+0x28c/0xa60 > [] ? hrtimer_get_res+0x50/0x50 > [] do_group_exit+0x3f/0xa0 > [] get_signal_to_deliver+0x1d0/0x6d0 > [] do_signal+0x57/0x6b0 > [] ? futex_wait_queue_me+0xa2/0x120 > [] ? __do_page_fault+0x183/0x470 > [] do_notify_resume+0x5f/0xb0 > [] int_signal+0x12/0x17 > > BUG: Bad rss-counter state mm:8820136b5dc0 idx:1 val:512 > > The pmd entry show that it is still a thp. but It fails to check and clear > the pmd. > hence, page fault will produce a new page for pmd when accessing the page, > which thp count will reduplicative increase. > > Thanks, > zhong jiang
Re: [PATCH 1/2] rtc:rtc-digicolor: Use PTR_ERR_OR_ZERO to replace the open code
On 2018/8/15 0:15, Alexandre Belloni wrote: > Hi, > > On 13/08/2018 19:31:24+0800, zhong jiang wrote: >> PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So >> just replace them rather than duplicating its implement. >> >> Signed-off-by: zhong jiang >> --- >> drivers/rtc/rtc-digicolor.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c >> index b253bf1..fd6850c 100644 >> --- a/drivers/rtc/rtc-digicolor.c >> +++ b/drivers/rtc/rtc-digicolor.c >> @@ -202,10 +202,8 @@ static int __init dc_rtc_probe(struct platform_device >> *pdev) >> platform_set_drvdata(pdev, rtc); >> rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name, >> &dc_rtc_ops, THIS_MODULE); >> -if (IS_ERR(rtc->rtc_dev)) >> -return PTR_ERR(rtc->rtc_dev); >> >> -return 0; >> +return PTR_ERR_OR_ZERO(rtc->rtc_dev); > As many other maintainers, I don't find that kind of change useful and > I'm not taking them unless there are other improvements in the driver. > > Hi, Alexandre The issue is detected with the help of Coccinelle. It simplify the code with specific function rather than duplicating its implementation. The patch clean up the code. of course, it is not a bug. if you do not care about it. I am ok with that. Thanks, zhong jiang
Re: [PATCH 1/2] phy:phy-brcm-us: Use PTR_ERR_OR_ZERO to replace the open code
On 2018/8/16 7:23, Florian Fainelli wrote: > On 08/13/2018 04:24 AM, zhong jiang wrote: >> PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So >> just replace them rather than duplicating its implement. > Subject should be: > > phy: phy-brcm-usb: Use PTR_ERR_OR_ZERO to replace open coded version Thank you for tips. It is my mistake.:-[ Sincerely, zhong jiang > Thank you > >> Signed-off-by: zhong jiang >> --- >> drivers/phy/broadcom/phy-brcm-usb.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/phy/broadcom/phy-brcm-usb.c >> b/drivers/phy/broadcom/phy-brcm-usb.c >> index d1dab36..f59b1dc 100644 >> --- a/drivers/phy/broadcom/phy-brcm-usb.c >> +++ b/drivers/phy/broadcom/phy-brcm-usb.c >> @@ -372,10 +372,8 @@ static int brcm_usb_phy_probe(struct platform_device >> *pdev) >> clk_disable(priv->usb_30_clk); >> >> phy_provider = devm_of_phy_provider_register(dev, brcm_usb_phy_xlate); >> -if (IS_ERR(phy_provider)) >> -return PTR_ERR(phy_provider); >> >> -return 0; >> +return PTR_ERR_OR_ZERO(phy_provider); >> } >> >> #ifdef CONFIG_PM_SLEEP >> >
Re: [PATCH] arm/mach-at91/pm: Do not double put the device node
On 2018/8/16 17:32, Claudiu Beznea wrote:
> Hi Alexandre,
>
> On 14.08.2018 15:59, Alexandre Belloni wrote:
>> On 14/08/2018 09:54:56+0800, zhong jiang wrote:
>>> Device node iterators put the previous value of the index variable,
>>> so an explicit put causes a double put.
>>>
>> While for_each_matching_node_and_match will get and put the node
>> properly, there is also a call to of_find_device_by_node that will get a
>> reference to the node.
>>
> Looking through of_find_device_by_node() it seems that a put_device() on the
> struct device member of the returned struct platform_device has to be called
> instead of of_node_put().
>
> of_find_device_by_node() calls bus_find_device():
>
> dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
>
> the match function, of_dev_node_match(), is just as follows:
>
> static int of_dev_node_match(struct device *dev, void *data)
>
> {
>
> return dev->of_node == data;
>
> }
>
>
> but bus_find_device() takes a reference to the struct device returned in case
> it
> founds a match, via get_device():
>
> struct device *bus_find_device(struct bus_type *bus,
>
>struct device *start, void *data,
>
>int (*match)(struct device *dev,
> void*data))
> {
>
> struct klist_iter i;
>
> struct device *dev;
>
>
>
> if (!bus || !bus->p)
>
> return NULL;
>
>
>
> klist_iter_init_node(&bus->p->klist_devices, &i,
>
> (start ? &start->p->knode_bus : NULL));
>
> while ((dev = next_device(&i)))
>
> if (match(dev, data) && get_device(dev))
>
> break;
>
> klist_iter_exit(&i);
>
> return dev;
>
> }
>
>
> So, I think a put_device(&pdev->dev) has to be called in at91_pm_config_ws()
> instead of of_node_put(np). My bad!
Yes, you're right. Thanks, Claudiu. I will repost in v2.
Sincerely,
zhong jiang
> Thank you,
> Claudiu Beznea
>
>>> I detect the issue with the help of Coccinelle.
>>>
>>> Signed-off-by: zhong jiang
>>> ---
>>> arch/arm/mach-at91/pm.c | 5 +
>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>>> index 32fae4d..a5ec35f 100644
>>> --- a/arch/arm/mach-at91/pm.c
>>> +++ b/arch/arm/mach-at91/pm.c
>>> @@ -143,15 +143,12 @@ static int at91_pm_config_ws(unsigned int pm_mode,
>>> bool set)
>>>
>>> /* Check if enabled on SHDWC. */
>>> if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
>>> - goto put_node;
>>> + continue;
>>>
>>> mode |= wsi->pmc_fsmr_bit;
>>> if (wsi->set_polarity)
>>> polarity |= wsi->pmc_fsmr_bit;
>>> }
>>> -
>>> -put_node:
>>> - of_node_put(np);
>>> }
>>>
>>> if (mode) {
>>> --
>>> 1.7.12.4
>>>
> .
>
[PATCH] arm: pm: call put_device instead of of_node_put in at91_pm_config_ws
of_find_device_by_node takes a reference to the struct device when it
finds a match via get_device. but it fails to put_device in
at91_pm_config_ws, for_each_matching_node_and_match will get and put
the node properly, there is no need to call the of_put_node. Therefore,
just call put_device instead of of_node_put in at91_pm_config_ws.
Fixes: d7484f5c6b3b ("ARM: at91: pm: configure wakeup sources for ULP1 mode")
Suggested-by: Claudiu Beznea
Signed-off-by: zhong jiang
---
arch/arm/mach-at91/pm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 32fae4d..6574f00 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -143,15 +143,15 @@ static int at91_pm_config_ws(unsigned int pm_mode, bool
set)
/* Check if enabled on SHDWC. */
if (wsi->shdwc_mr_bit && !(val & wsi->shdwc_mr_bit))
- goto put_node;
+ goto put_device;
mode |= wsi->pmc_fsmr_bit;
if (wsi->set_polarity)
polarity |= wsi->pmc_fsmr_bit;
}
-put_node:
- of_node_put(np);
+put_device:
+ put_device(&pdev->dev);
}
if (mode) {
--
1.7.12.4
[PATCHv2 2/2] phy:phy-lantiq-rcu-usb2: Use PTR_ERR_OR_ZERO to replace the open coded version
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So just replace them rather than duplicating its implement. Signed-off-by: zhong jiang --- drivers/phy/lantiq/phy-lantiq-rcu-usb2.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c b/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c index 986224f..a918c5b 100644 --- a/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c +++ b/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c @@ -196,10 +196,8 @@ static int ltq_rcu_usb2_of_parse(struct ltq_rcu_usb2_priv *priv, } priv->phy_reset = devm_reset_control_get_optional(dev, "phy"); - if (IS_ERR(priv->phy_reset)) - return PTR_ERR(priv->phy_reset); - return 0; + return PTR_ERR_OR_ZERO(priv->phy_reset); } static int ltq_rcu_usb2_phy_probe(struct platform_device *pdev) -- 1.7.12.4
[PATCHv2 0/2] phy: Use PTR_ERR_OR_ZERO to replace the open coded version
The issue is detected with the help of Coccinelle. v1->v2: - According to Florian's suggestion, change the subject of the patch. zhong jiang (2): phy:phy-brcm-usb: Use PTR_ERR_OR_ZERO to replace the open coded version phy:phy-lantiq-rcu-usb2: Use PTR_ERR_OR_ZERO to replace the open coded version drivers/phy/broadcom/phy-brcm-usb.c | 4 +--- drivers/phy/lantiq/phy-lantiq-rcu-usb2.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) -- 1.7.12.4
[PATCHv2 1/2] phy:phy-brcm-usb: Use PTR_ERR_OR_ZERO to replace the open coded version
PTR_ERR_OR_ZERO has implemented the if(IS_ERR(...)) + PTR_ERR, So just replace them rather than duplicating its implement. Signed-off-by: zhong jiang --- drivers/phy/broadcom/phy-brcm-usb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c index d1dab36..f59b1dc 100644 --- a/drivers/phy/broadcom/phy-brcm-usb.c +++ b/drivers/phy/broadcom/phy-brcm-usb.c @@ -372,10 +372,8 @@ static int brcm_usb_phy_probe(struct platform_device *pdev) clk_disable(priv->usb_30_clk); phy_provider = devm_of_phy_provider_register(dev, brcm_usb_phy_xlate); - if (IS_ERR(phy_provider)) - return PTR_ERR(phy_provider); - return 0; + return PTR_ERR_OR_ZERO(phy_provider); } #ifdef CONFIG_PM_SLEEP -- 1.7.12.4
Re: [PATCH] drivers/thermal/tegra: fix a doule free devce node
On 2018/8/14 14:39, Daniel Lezcano wrote:
> On 09/08/2018 15:40, zhong jiang wrote:
>> Device node iterators will get the return node. Meawhile, It is
>> also put the previous device node. An explicit put will cause
>> a double put.
> What about:
>
> Subject: drivers/thermal/tegra: Fix a double free on the device node
>
> "The function 'for_each_child_of_node' iterates over the node list by
> dropping the of_node reference of the previous node.
>
> Calling of_node_put() on the iterator is pointless and leads to an
> inconsistent refcounting in addition to a double free. Remove it."
Hi, Daniel
Have you picked up the patch or should I repost the patch with above commit
message that you have suggested?
Thanks,
zhong jiang
>> Signed-off-by: zhong jiang
>> ---
>> drivers/thermal/tegra/soctherm.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/thermal/tegra/soctherm.c
>> b/drivers/thermal/tegra/soctherm.c
>> index ed28110..3aa55c9 100644
>> --- a/drivers/thermal/tegra/soctherm.c
>> +++ b/drivers/thermal/tegra/soctherm.c
>> @@ -980,7 +980,6 @@ static void soctherm_init_hw_throt_cdev(struct
>> platform_device *pdev)
>> tcd = thermal_of_cooling_device_register(np_stcc,
>> (char *)name, ts,
>> &throt_cooling_ops);
>> -of_node_put(np_stcc);
>> if (IS_ERR_OR_NULL(tcd)) {
>> dev_err(dev,
>> "throttle-cfg: %s: failed to register cooling
>> device\n",
>>
>
[PATCH] misc: remove meanless null check before kfree
kfree has taken null pointer into account. so check the null pointer
before kfree is meanless.
Signed-off-by: zhong jiang
---
drivers/misc/sgi-xp/xpc_partition.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/misc/sgi-xp/xpc_partition.c
b/drivers/misc/sgi-xp/xpc_partition.c
index 0c3ef6f..3eba1c4 100644
--- a/drivers/misc/sgi-xp/xpc_partition.c
+++ b/drivers/misc/sgi-xp/xpc_partition.c
@@ -98,8 +98,7 @@
len = L1_CACHE_ALIGN(len);
if (len > buf_len) {
- if (buf_base != NULL)
- kfree(buf_base);
+ kfree(buf_base);
buf_len = L1_CACHE_ALIGN(len);
buf = xpc_kmalloc_cacheline_aligned(buf_len, GFP_KERNEL,
&buf_base);
--
1.7.12.4
[PATCH] drviers: intel_ips: remove the unneeded null pointer check before debugfs_remove_recursive
debugfs_remove_recursive has taken the null pointer into account.
just remove the null check before debugfs_remove_recursive.
Signed-off-by: zhong jiang
---
drivers/platform/x86/intel_ips.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
index c5ece7e..1c3127c 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -1311,8 +1311,7 @@ static int ips_debugfs_open(struct inode *inode, struct
file *file)
static void ips_debugfs_cleanup(struct ips_driver *ips)
{
- if (ips->debug_root)
- debugfs_remove_recursive(ips->debug_root);
+ debugfs_remove_recursive(ips->debug_root);
return;
}
--
1.7.12.4
