On 2018/2/17 3:57, Jason Gunthorpe wrote: > On Sun, Feb 11, 2018 at 08:40:33PM +0800, Yixian Liu wrote: >> This patch updates to support rq record doorbell in the >> user space driver. >> >> Signed-off-by: Yixian Liu <liuyixian@xxxxxxxxxx> >> Signed-off-by: Lijun Ou <oulijun@xxxxxxxxxx> >> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@xxxxxxxxxx> >> Signed-off-by: Shaobo Xu <xushaobo2@xxxxxxxxxx> >> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx> >> Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > You should't add those Reviwed-by tags unless someone actually sends > you a tag.. Thanks, I see. >> +#define BIT_CNT_PER_BYTE 8 > > I'm getting tired of seeing people open code bitmaps. > > Can you please add and use ccan/bitmap for this? > > https://github.com/rustyrussell/ccan/blob/master/ccan/bitmap/ > > If this is unclear I can help you > Hi Jason: It is a good suggestion! I have already adapted the patch set and send out v5. As current rdma-core/ccan has no bitmap related files, thus they needed to be added. I have done that work in a separate patch [1/3]. If it is unsuitable for me to submit this patch, could you please do this work for me? Thanks again! >> + /* alloc db page */ >> + page_size = (uint32_t)to_hr_dev(ctx->ibv_ctx.context.device)->page_size; >> + page = (struct hns_roce_db_page *)calloc(1, sizeof(*page)); > > And please go through and clean up all the extra casting, none of > this is necessary. > >> +static void hns_roce_clear_db_page(struct hns_roce_db_page *page) >> +{ >> + if (!page) { >> + fprintf(stderr, "error clear: db page is NULL!\n"); >> + return; > > No prints in drivers. Use assert() Okay, will fix it. > > Same sort of comments on other patches > > Jason > Okay, I will pay my attention to those problems. > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
