Wed, Mar 09, 2016 at 12:18:06PM CET, ouli...@huawei.com wrote: >Hi Jiri Pirko, thanks for reviewing >On 2016/3/4 17:16, Jiri Pirko wrote: >> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote: >> >> <snip> >> >>> +int hns_roce_buf_alloc( >>> + struct hns_roce_dev *hr_dev, >>> + int size, int max_direct, >>> + struct hns_roce_buf *buf) >> >> <snip> >> >>> + >>> + pages = >>> + kmalloc(sizeof(*pages) * buf->nbufs, >>> + GFP_KERNEL); >> >> <snip> >> >>> + >>> + buf->direct.buf = vmap( >>> + pages, buf->nbufs, VM_MAP, >>> + PAGE_KERNEL); >> >> <snip> >> >>> + if ( >>> + event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID && >>> + event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR && >>> + event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) { >>> + dev_err(&hr_dev->pdev->dev, >>> + "hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n", >>> + event_type, hr_cq->cqn); >>> + return; >>> + } >> >> Although checkpatch does not complain, I find this semi-random adding of >> newlines quite odd. >> > Really, the question you mentioned exit in many location in currently > patch. I done it >in order to make it complain checkpatch and linux norms. Now, I have checked >and adjust it >properly combined to checkpatch > I will send a new patch in future. if not modified in some locations, it > have to violate >checkpatch once modified and is unable to adjust it better. About these, have >you best strategy?
I'm not sure what violation you are talking about. I'm just simply suggesting to change: buf->direct.buf = vmap( pages, buf->nbufs, VM_MAP, PAGE_KERNEL); to: buf->direct.buf = vmap(pages, buf->nbufs, VM_MAP, PAGE_KERNEL); and to change: pages = kmalloc(sizeof(*pages) * buf->nbufs, GFP_KERNEL); to: pages = kmalloc(sizeof(*pages) * buf->nbufs, GFP_KERNEL);