Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: "Liuyixian (Eason)" <liuyixian@huawei.com>
Cc: dledford@redhat.com, leon@kernel.org, linux-rdma@vger.kernel.org,
	linuxarm@huawei.com
Subject: Re: [PATCH v5 for-next 1/2] RDMA/hns: Add the workqueue framework for flush cqe handler
Date: Mon, 13 Jan 2020 10:04:42 -0400
Message-ID: <20200113140442.GA9861@ziepe.ca> (raw)
In-Reply-To: <65fb928c-5f85-02f9-c5ac-06037b3fe967@huawei.com>

On Mon, Jan 13, 2020 at 07:26:45PM +0800, Liuyixian (Eason) wrote:
> 
> 
> On 2020/1/10 23:26, Jason Gunthorpe wrote:
> > On Sat, Dec 28, 2019 at 11:28:54AM +0800, Yixian Liu wrote:
> >> +void init_flush_work(struct hns_roce_dev *hr_dev, struct hns_roce_qp *hr_qp)
> >> +{
> >> +	struct hns_roce_work *flush_work;
> >> +
> >> +	flush_work = kzalloc(sizeof(struct hns_roce_work), GFP_ATOMIC);
> >> +	if (!flush_work)
> >> +		return;
> > 
> > You changed it to only queue once, so why do we need the allocation
> > now? That was the whole point..
> 
> Hi Jason,
> 
> The flush work is queued **not only once**. As the flag being_pushed is set to 0 during
> the process of modifying qp like this:
> 	hns_roce_v2_modify_qp {
> 		...
> 		if (new_state == IB_QPS_ERR) {
> 			spin_lock_irqsave(&hr_qp->sq.lock, sq_flag);
> 			...
> 			hr_qp->state = IB_QPS_ERR;
> 			hr_qp->being_push = 0;
> 			...
> 		}
> 		...
> 	}
> which means the new updated PI value needs to be updated with initializing a new flush work.
> Thus, maybe there are two flush work in the workqueue. Thus, we still need the allocation here.

I don't see how you should get two? One should be pending until the
modify is done with the new PI, then once the PI is updated the same
one should be re-queued the next time the PI needs changing.

Jason

  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-28  3:28 [PATCH v5 for-next 0/2] " Yixian Liu
2019-12-28  3:28 ` [PATCH v5 for-next 1/2] " Yixian Liu
2020-01-10 15:26   ` Jason Gunthorpe
2020-01-11  9:49     ` Liuyixian (Eason)
2020-01-13 11:26     ` Liuyixian (Eason)
2020-01-13 14:04       ` Jason Gunthorpe [this message]
2020-01-15  9:36         ` Liuyixian (Eason)
2020-01-15  9:39         ` Liuyixian (Eason)
2019-12-28  3:28 ` [PATCH v5 for-next 2/2] RDMA/hns: Delayed flush cqe process with workqueue Yixian Liu

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200113140442.GA9861@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=dledford@redhat.com \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=liuyixian@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git