Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH rdma-core] Add a .travis.yml file
From: Bart Van Assche @ 2016-10-05 16:39 UTC (permalink / raw)
  To: Leon Romanovsky, Jason Gunthorpe
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161005115132.GE9282-2ukJVAZIZ/Y@public.gmane.org>

On 10/05/2016 04:51 AM, Leon Romanovsky wrote:
> On Wed, Sep 28, 2016 at 05:28:57PM -0600, Jason Gunthorpe wrote:
>> Necessary to use the Travis CI service.
>>
>> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>> ---
>>  .travis.yml | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>  create mode 100644 .travis.yml
>>
>> Now that travis is enabled on repo, we need a yml file to make it run.
>>
>> Here is a basic one.
>>
>> I don't test with distros as old as precise and I don't expect that glibc to
>> be new enough to build this (eg missing timerfd, O_CLOEXEC, etc), and the
>> cmake certainly isn't. So we use the trusty 'beta' environment and install
>> gcc 6.2.
>
> I'm using 16.04 and it has gcc 5.4.0 as a default. Don't you think that
> 6.2 is too "new" for us and other distributions?

Hi Leon,

The rolling release distro that is running on my laptop includes gcc 6.2.1.

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

^ permalink raw reply

* Re: [PATCH 8/9] Remove most checks of __BYTE_ORDER
From: Jason Gunthorpe @ 2016-10-05 17:33 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Steve Wise, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	'Tatyana Nikolova', 'Hal Rosenstock',
	'Doug Ledford', 'Yishai Hadas',
	'Devesh Sharma', 'Sean Hefty',
	'Bart Van Assche'
In-Reply-To: <20161005153701.GA12295-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

On Wed, Oct 05, 2016 at 08:37:01AM -0700, Christoph Hellwig wrote:
> On Thu, Sep 29, 2016 at 04:28:02PM -0600, Jason Gunthorpe wrote:
> > In the kernel the swapping behavior of 'writel' is architecture (and
> > sometimes even platform) specific. Looking at the kernel headers
> > suggests that BE PPC needs an explicit swap and BE ARM does not (eg
> > the hardware swaps). Maybe. There are a lot of layers of macros in
> > this area..
> 
> read{s,l,q} and write{s,l,q} in the kernel always perform an implicit
> bytestap, that is the device is expected to use a LE layout.  This
> is not architecture specific, otherwise hell would break lose.

Right, stated differently, the kernel requires that writel()/etc
always produce the same PCI-E packet on the wire. (eg writel(1)
produces a TLP with bit 0 of the data payload set)

But each arch implements its own set of instructions to get there,
that is what I mean by architecture specific.

> But none of this has an affect on userspace mappings of registers, as
> those don't go through the kernel read{s,l,q} and write{s,l,q}
> helpers,

Not going through the kernel's writel is the whole problem. The writel
helper generates the arch-specific instruction sequence required to
issue generate the required PCI-E packet.

Today (at best, some drivers do not even do this) our userspace
assumes all archs implement writel as:

   *(u32 *)reg = cpu_to_le32(val);

Which is a good start, but not portable to every arch the kernel
supports.

> Btw, one thing that would be useful is to introduce endiane annoatations
> for sparse in rdma-core.   I've added these to a few userspace projects
> already, here is the most recent example:
> 
> https://github.com/linux-nvme/nvme-cli/commit/126b8beb35b76fe9f61b137e34aea66b64c07b25

Yeah, it would be nice to get that working too. I guess we need to
standardize on the cpu_to_xx macro style as a first step?

The __iomem annotation would be nice as well.

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

^ permalink raw reply

* Re: [PATCH v2 6/7] SRP transport: Port srp_wait_for_queuecommand() to scsi-mq
From: Sagi Grimberg @ 2016-10-05 17:38 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <bedf4b45-00b0-6844-047d-f1e05bb1092f-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>


> +static void srp_mq_wait_for_queuecommand(struct Scsi_Host *shost)
> +{
> +	struct scsi_device *sdev;
> +	struct request_queue *q;
> +
> +	shost_for_each_device(sdev, shost) {
> +		q = sdev->request_queue;
> +
> +		blk_mq_quiesce_queue(q);
> +		blk_mq_resume_queue(q);
> +	}
> +}
> +

This *should* live in scsi_lib.c. I suspect that
various drivers would really want this functionality.

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

^ permalink raw reply

* Re: [PATCH v2 7/7] [RFC] nvme: Fix a race condition
From: Sagi Grimberg @ 2016-10-05 17:40 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <ff412bfb-548c-c4fc-5180-78ede8a19ef2@sandisk.com>


> Avoid that nvme_queue_rq() is still running when nvme_stop_queues()
> returns. Untested.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Sagi Grimberg <sagi@grimberg.me>

Bart this looks really good! and possibly fixes an issue
I've been chasing with fabrics a while ago. I'll take it
for testing but you can add my:

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-05 17:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Hannes Reinecke, Johannes Thumshirn, linux-rdma, linux-scsi
In-Reply-To: <20161004114642.GA2377@infradead.org>

We are not able to identify the patch that you mentioned from Lee, can
you give us a commit or a link to the patch?

Thanks,
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Tue, Oct 4, 2016 at 5:46 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Oct 04, 2016 at 11:11:18AM +0200, Hannes Reinecke wrote:
>> Hmm. Looking at the code it looks as we might miss some calls to
>> 'complete'. Can you try with the attached patch?
>
> That only looks slightly better than the original.  What this really
> needs is a waitqueue and and waitevent on sess->ncon.  Although
> that will need a bit more refactoring around that code.  There also
> are a few more ovbious issues around it, e.g. iscsit_close_connection
> needs to use atomic_dec_and_test on sess->nconn instead of having
> separate atomic_dec and atomic_read calls, and a lot of the 0 or 1
> atomic_ts in this code should be replaced with atomic bitops.
>
> Btw, there also was a fix from Lee in this area that added a missing
> wakeup, make sure your tree already has that.

^ permalink raw reply

* Re: [PATCH v6 02/16] IB/pvrdma: Add user-level shared functions
From: Adit Ranadive @ 2016-10-05 17:43 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	pv-drivers-pghWNbHTmq7QT0dZR+AlfA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
	asarwade-pghWNbHTmq7QT0dZR+AlfA,
	georgezhang-pghWNbHTmq7QT0dZR+AlfA,
	bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20161005134254.GH9282-2ukJVAZIZ/Y@public.gmane.org>

On Wed, Oct 05, 2016 at 4:42:54PM +0300, Leon Romanovsky wrote:
> On Sun, Oct 02, 2016 at 07:10:22PM -0700, Adit Ranadive wrote: > > We share some common structures with the user-level driver. This patch adds > > those structures and shared functions to traverse the QP/CQ rings. > > > > Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> > > Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> > > Reviewed-by: George Zhang <georgezhang-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> > > Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> > > Reviewed-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> > > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> > > --- > > Changes v5->v6: > >  - Removed the pvrdma-uapi.h file and moved common structures into > >  pvrdma-abi.h. > >  - Moved enums and structs common to user-level and kernel driver into > >  pvrdma-abi.h. > >  - Changed _exp_ to _ex_ for extended structures. > > > > Changes v4->v5: > >  - Moved pvrdma_uapi.h and pvrdma_user.h into common UAPI folder. > >  - Renamed to pvrdma-uapi.h and pvrdma-abi.h respectively. > >  - Prefixed unsigned vars with __. > > > > Changes v3->v4: > >  - Moved pvrdma_sge into
pvrdma_uapi.h > > --- > >  include/uapi/rdma/Kbuild       |   2 + > >  include/uapi/rdma/pvrdma-abi.h | 289 +++++++++++++++++++++++++++++++++++++++++ > >  2 files changed, 291 insertions(+) > >  create mode 100644 include/uapi/rdma/pvrdma-abi.h > > > > diff --git a/include/uapi/rdma/Kbuild b/include/uapi/rdma/Kbuild > > index ac62908..04357ba 100644 > > --- a/include/uapi/rdma/Kbuild > > +++ b/include/uapi/rdma/Kbuild > > @@ -8,3 +8,5 @@ header-y += rdma_netlink.h > >  header-y += rdma_user_cm.h > >  header-y += hfi/ > >  header-y += rdma_user_rxe.h > > +header-y += pvrdma-abi.h > > +header-y += pvrdma-uapi.h > > You forgot to drop this line.

Yes, sorry about that. I mentioned this to Doug already. He didnt want me
to do a respin just for this though.

Thanks,
Adit

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

^ permalink raw reply

* Re: [PATCH v2 3/7] [RFC] nvme: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Sagi Grimberg @ 2016-10-05 17:43 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <9faa7607-eb3e-ce41-b09e-1e10492a5f45-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

> Make nvme_requeue_req() check BLK_MQ_S_STOPPED instead of
> QUEUE_FLAG_STOPPED. Remove the QUEUE_FLAG_STOPPED manipulations
> that became superfluous because of this change. This patch fixes
> a race condition: using queue_flag_clear_unlocked() is not safe
> if any other function that manipulates the queue flags can be
> called concurrently, e.g. blk_cleanup_queue(). Untested.

This looks good to me, but I know keith had all sort of
creative ways to challenge this are so I'd wait for his
input...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 8/9] Remove most checks of __BYTE_ORDER
From: Jason Gunthorpe @ 2016-10-05 17:44 UTC (permalink / raw)
  To: Yishai Hadas
  Cc: Steve Wise, Steve Wise, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	Tatyana Nikolova, Hal Rosenstock, Doug Ledford, Devesh Sharma,
	Sean Hefty, Bart Van Assche,
	yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <fd7d47c8-6a06-6169-b840-abfbc239ad75-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

On Wed, Oct 05, 2016 at 06:16:45PM +0300, Yishai Hadas wrote:

> Upon QP creation the code uses htonl for setting qp->doorbell_qpn (see
> mlx4_create_qp_ex), later on this value is used without any change upon
> writing a door bell. (see mmio_writel). No PCI swap is expected.

Thanks, so the mlx drivers are doing what the kernel calls writel_be()?

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

^ permalink raw reply

* Re: [PATCH v2 1/7] blk-mq: Introduce blk_mq_queue_stopped()
From: Sagi Grimberg @ 2016-10-05 17:50 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <26c7db22-bac8-6d8c-ba6e-3cbf0cec3368@sandisk.com>

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Christoph Hellwig @ 2016-10-05 18:03 UTC (permalink / raw)
  To: Robert LeBlanc
  Cc: Christoph Hellwig, Hannes Reinecke, Johannes Thumshirn,
	linux-rdma, linux-scsi
In-Reply-To: <CAANLjFrNz45nizmPa7XH4Hpw7e-KScGCsaMpXWG0BCNhuXxOCg@mail.gmail.com>

Hi Robert,

I actually got the name wrong, the patch wasn't from Lee, but from Zhu,
another SuSE engineer.  This is the one:

http://www.spinics.net/lists/target-devel/msg13463.html

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Sagi Grimberg @ 2016-10-05 18:14 UTC (permalink / raw)
  To: Bart Van Assche, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <a11bb09a-6207-f4a8-cfd4-4fe5627c1700-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>


> Hello Ming,
>
> Can you have a look at the attached patch? That patch uses an srcu read
> lock for all queue types, whether or not the BLK_MQ_F_BLOCKING flag has
> been set. Additionally, I have dropped the QUEUE_FLAG_QUIESCING flag.
> Just like previous versions, this patch has been tested.

Hey Bart,

Do we care about the synchronization of queue_rq and/or
blk_mq_run_hw_queue of the hctx is not stopped?

I'm wandering if we can avoid introducing new barriers in the
submission path of its not absolutely needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-05 18:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Hannes Reinecke, Johannes Thumshirn, linux-rdma, linux-scsi
In-Reply-To: <20161005180305.GA2900@infradead.org>

Thanks, we will apply that too. We'd like to get this stable. We'll
report back on what we find with these patches.
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Wed, Oct 5, 2016 at 12:03 PM, Christoph Hellwig <hch@infradead.org> wrote:
> Hi Robert,
>
> I actually got the name wrong, the patch wasn't from Lee, but from Zhu,
> another SuSE engineer.  This is the one:
>
> http://www.spinics.net/lists/target-devel/msg13463.html

^ permalink raw reply

* Re: RDMA developer gatherings around Kernel Summit and Linux Plumbers in Santa Fe
From: Jason Gunthorpe @ 2016-10-05 18:31 UTC (permalink / raw)
  To: Atchley, Scott
  Cc: Christoph Hellwig, Christoph Lameter, Linux RDMA Mailing List,
	ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	john.fleck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Doug Ledford,
	Coulter, Susan K, shewa-YOWKrPYUwWM@public.gmane.org
In-Reply-To: <96C6AF13-B968-41C2-9AB3-CAB5F91D72A1-1Heg1YXhbW8@public.gmane.org>

On Wed, Oct 05, 2016 at 01:49:56PM +0000, Atchley, Scott wrote:

> I am trying to understand the issue from both points of view. Again,
> I ask you to help me understand how requesting dual-licensing hurts
> the adoption of RDMA technology. I can understand the vendor???s
> concerns about having to write software twice for GPL and non-GPL
> environments.

The opposing point of view is that every time an opensource project is
forked or taken propriety by a company the opensource project itself
suffers. (and this is by no means a universal view, but let us
explore it)

A good example of this harm in our community would be opensm. Many
sites are now running a build of 'opensm' that does not come with
source.

>From the OFA perspective, many of the companies using the code outside
Linux are not OFA members, do not contribute financially, and do not
contribute their bugfixes or other code. Yet, they get a 'free ride'
when the OFA encourages Linux-focused member companies to use the dual
license.

I can understand why the OFA adopted this position at the start, but
IMHO, it is time to stop. The OFA should not be spending its limited
resources to benefit the non-Linux community.

Further, it is hard to see why the OFA board should burn what little
good will it has in the Linux community by acting to the benifit of a
nebulous sub-group of non-Linux members (that may not even exist today).

In the end I hope the bylaws will be revised to allow for
GPLv2/LGPLv2-only as a legtimiate license choice for OFA projects.

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

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-10-05 19:05 UTC (permalink / raw)
  To: Sagi Grimberg, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <09238e87-7d60-89c9-98b0-e51caa0c0839@grimberg.me>

On 10/05/2016 11:14 AM, Sagi Grimberg wrote:
>> Hello Ming,
>>
>> Can you have a look at the attached patch? That patch uses an srcu read
>> lock for all queue types, whether or not the BLK_MQ_F_BLOCKING flag has
>> been set. Additionally, I have dropped the QUEUE_FLAG_QUIESCING flag.
>> Just like previous versions, this patch has been tested.
>
> Hey Bart,
>
> Do we care about the synchronization of queue_rq and/or
> blk_mq_run_hw_queue of the hctx is not stopped?
>
> I'm wandering if we can avoid introducing new barriers in the
> submission path of its not absolutely needed.

Hello Sagi,

I'm not sure whether the new blk_quiesce_queue() function is useful 
without stopping all hardware contexts first. In other words, in my view 
setting BLK_MQ_F_BLOCKING flag before calling blk_quiesce_queue() is 
sufficient and I don't think that a new QUEUE_FLAG_QUIESCING flag is 
necessary.

Bart.

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Sagi Grimberg @ 2016-10-05 19:10 UTC (permalink / raw)
  To: Bart Van Assche, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <6d612be9-7ba7-888e-f9c6-f8aa4ef1606a@sandisk.com>


>>> Hello Ming,
>>>
>>> Can you have a look at the attached patch? That patch uses an srcu read
>>> lock for all queue types, whether or not the BLK_MQ_F_BLOCKING flag has
>>> been set. Additionally, I have dropped the QUEUE_FLAG_QUIESCING flag.
>>> Just like previous versions, this patch has been tested.
>>
>> Hey Bart,
>>
>> Do we care about the synchronization of queue_rq and/or
>> blk_mq_run_hw_queue of the hctx is not stopped?
>>
>> I'm wandering if we can avoid introducing new barriers in the
>> submission path of its not absolutely needed.
>
> Hello Sagi,

Hey Bart,

>
> I'm not sure whether the new blk_quiesce_queue() function is useful
> without stopping all hardware contexts first. In other words, in my view
> setting BLK_MQ_F_BLOCKING flag before calling blk_quiesce_queue() is
> sufficient and I don't think that a new QUEUE_FLAG_QUIESCING flag is
> necessary.

I was referring to weather we can take srcu in the submission path
conditional of the hctx being STOPPED?

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: Update Intel RDMA RNIC driver maintainers
From: Jason Gunthorpe @ 2016-10-05 19:12 UTC (permalink / raw)
  To: Faisal Latif
  Cc: Henry Orosco, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <20161005153530.GA11148@flatif-MOBL1>

On Wed, Oct 05, 2016 at 10:35:30AM -0500, Faisal Latif wrote:
> On Wed, Oct 05, 2016 at 07:30:54AM -0700, Henry Orosco wrote:
> Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  MAINTAINERS | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index cc77b9a..32f535e 100644
> > +++ b/MAINTAINERS
> > @@ -6188,6 +6188,7 @@ R:     Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > +R:     Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  L:     linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >  S:     Supported
> >  F:     drivers/infiniband/hw/i40iw/
> 
> Acked-by: Faisal Latif <faisal.latif-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Are any of you going to continue supporting the user space component
that now lives in rdma-core?

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

^ permalink raw reply

* Re: [PATCH rdma-core] Remove unused HIDDEN gcc attribute
From: Jason Gunthorpe @ 2016-10-05 19:23 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475665714-5885-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Wed, Oct 05, 2016 at 02:08:34PM +0300, Leon Romanovsky wrote:
> The attribute HIDDEN was declared is almost every provider, but it wasn't
> used, except in libibverbs. Let's remove it.

There is no reason for libibverbs to use this either, please remove
it completely.

All libraries and providers use a linker map file which superceeds
the hidden attribute.

Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

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

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-10-05 21:08 UTC (permalink / raw)
  To: Sagi Grimberg, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <732bf5d6-d7a8-6ff0-6501-8a3d46f12fab-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>

On 10/05/2016 12:11 PM, Sagi Grimberg wrote:
> I was referring to weather we can take srcu in the submission path
> conditional of the hctx being STOPPED?

Hello Sagi,

Regarding run-time overhead:
* rcu_read_lock() is a no-op on CONFIG_PREEMPT_NONE kernels and is
   translated into preempt_disable() with preemption enabled. The latter
   function modifies a per-cpu variable.
* Checking BLK_MQ_S_STOPPED before taking an rcu or srcu lock is only
   safe if the BLK_MQ_S_STOPPED flag is tested in such a way that the
   compiler is told to reread the hctx flags (READ_ONCE()) and if the
   compiler and CPU are told not to reorder test_bit() with the
   memory accesses in (s)rcu_read_lock(). To avoid races
   BLK_MQ_S_STOPPED will have to be tested a second time after the lock
   has been obtained, similar to the double-checked-locking pattern.
* srcu_read_lock() reads a word from the srcu structure, disables
   preemption, calls __srcu_read_lock() and re-enables preemption. The
   latter function increments two CPU-local variables and triggers a
   memory barrier (smp_mp()).

Swapping srcu_read_lock() and the BLK_MQ_S_STOPPED flag test will make 
the code more complicated. Going back to the implementation that calls 
rcu_read_lock() if .queue_rq() won't sleep will result in an 
implementation that is easier to read and to verify. If I overlooked 
something, please let me know.

Bart.


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

^ permalink raw reply

* Re: [PATCH v2 6/7] SRP transport: Port srp_wait_for_queuecommand() to scsi-mq
From: Bart Van Assche @ 2016-10-05 21:51 UTC (permalink / raw)
  To: Sagi Grimberg, Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <8ce2910d-fde0-724f-a61f-8fbd7fbf499d@grimberg.me>

On 10/05/2016 10:38 AM, Sagi Grimberg wrote:
>> +static void srp_mq_wait_for_queuecommand(struct Scsi_Host *shost)
>> +{
>> +    struct scsi_device *sdev;
>> +    struct request_queue *q;
>> +
>> +    shost_for_each_device(sdev, shost) {
>> +        q = sdev->request_queue;
>> +
>> +        blk_mq_quiesce_queue(q);
>> +        blk_mq_resume_queue(q);
>> +    }
>> +}
>> +
>
> This *should* live in scsi_lib.c. I suspect that
> various drivers would really want this functionality.

Hello Sagi,

There are multiple direct blk_*() calls in other SCSI transport drivers. 
So my proposal is to wait with moving this code into scsi_lib.c until 
there is a second user of this code.

Bart.

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Ming Lei @ 2016-10-05 22:49 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Sagi Grimberg, Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <07628c2f-3a4e-b12e-663a-b0711ce20cc9-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

On Thu, Oct 6, 2016 at 5:08 AM, Bart Van Assche
<bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:
> On 10/05/2016 12:11 PM, Sagi Grimberg wrote:
>>
>> I was referring to weather we can take srcu in the submission path
>> conditional of the hctx being STOPPED?
>
>
> Hello Sagi,
>
> Regarding run-time overhead:
> * rcu_read_lock() is a no-op on CONFIG_PREEMPT_NONE kernels and is
>   translated into preempt_disable() with preemption enabled. The latter
>   function modifies a per-cpu variable.
> * Checking BLK_MQ_S_STOPPED before taking an rcu or srcu lock is only
>   safe if the BLK_MQ_S_STOPPED flag is tested in such a way that the
>   compiler is told to reread the hctx flags (READ_ONCE()) and if the
>   compiler and CPU are told not to reorder test_bit() with the
>   memory accesses in (s)rcu_read_lock(). To avoid races
>   BLK_MQ_S_STOPPED will have to be tested a second time after the lock
>   has been obtained, similar to the double-checked-locking pattern.
> * srcu_read_lock() reads a word from the srcu structure, disables
>   preemption, calls __srcu_read_lock() and re-enables preemption. The
>   latter function increments two CPU-local variables and triggers a
>   memory barrier (smp_mp()).

We can use srcu read lock for BLOCKING and rcu read lock for non-BLOCKING,
by putting *_read_lock() and *_read_unlock() into two wrappers, which
should minimize the cost of srcu read lock & unlock and the code is still easy
to read & verify.

>
> Swapping srcu_read_lock() and the BLK_MQ_S_STOPPED flag test will make the
> code more complicated. Going back to the implementation that calls
> rcu_read_lock() if .queue_rq() won't sleep will result in an implementation
> that is easier to read and to verify.

Yeah, I agree.

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

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-10-05 23:00 UTC (permalink / raw)
  To: Ming Lei
  Cc: Sagi Grimberg, Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <CACVXFVMwQnOuUukEMO8CyG3sPOg6DxuKAZx+5FT+LS+hGAcWVQ@mail.gmail.com>

On 10/05/2016 03:49 PM, Ming Lei wrote:
> We can use srcu read lock for BLOCKING and rcu read lock for non-BLOCKING,
> by putting *_read_lock() and *_read_unlock() into two wrappers, which
> should minimize the cost of srcu read lock & unlock and the code is still easy
> to read & verify.

Hello Ming,

The lock checking algorithms in the sparse and smatch static checkers 
are unable to deal with code of the type "if (condition) (un)lock()". So 
unless someone has a better proposal my preference is to use the 
approach from the patch at the start of this e-mail thread.

Thanks,

Bart.


^ permalink raw reply

* Re: [PATCH rdma-core] Improve global COPYING files
From: Leon Romanovsky @ 2016-10-06  4:30 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161005124939.GF9282-2ukJVAZIZ/Y@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]

On Wed, Oct 05, 2016 at 03:49:39PM +0300, Leon Romanovsky wrote:
> On Fri, Sep 23, 2016 at 11:34:21AM -0600, Jason Gunthorpe wrote:
> > On Fri, Sep 23, 2016 at 08:10:03PM +0300, Leon Romanovsky wrote:
> >
> > > I sure that this question will sound dumb for you, but can we unify all
> > > code under OFA's licenses (dual-license)? I don't feel comfortable with
> > > this multi-license situation.
> >
> > It is an excellent question.
> >
> > To do this we would need the OFA to talk to each of the member
> > companies and get them to sign some kind of legal change of copyright
> > document. Assuming all of the member companies agree, and all the
> > member companies are the exhaustive copyright owners then the code can
> > be placed under a single uniform license.
> >
> > As I understand it all OFA members were required to agree to use a
> > specific licensing scheme, including specific license text when they
> > signed the OFA membership agreement. What we are seeing here is that
> > the corporate legal side agreed to something but the developers made
> > small errors along the way, and those errors were later copied by
> > other developers and spread widely. So the OFA has a basis for
> > requests of this nature.
> >
> > For instance, a legal statement from Mellanox that all code they
> > contributed is available under the GPLv2 or *either* MIT or FreeBSD
> > license varient would allow immediately placing all Mellanox
> > copyrighted code under the single Default License. I expect this is
> > what Mellanox intended to do anyhow, the fact that ibverbs and all
> > their providers had an error in the COPYING file is simply
> > an unfortunate mistake.
> >
> > These sorts of license issues are typical in historical code bases. I
> > would say we are in pretty good shape, from what I can tell absolutely
> > everything is unambiguously licensed under at least the GPLv2, or a
> > compatible license.
> >
> > Almost everything is alternatively licensed under some kind of BSD
> > license. The notable exception is ipathverbs and rxe. It is also
> > unfortunate we have so many BSD variants.
> >
> > Further, almost all C code is licensed under the dual GPLv2/OpenIB.org
> > (MIT) license. The kernel is similar, almost all C code is using the
> > MIT variant. Hence my desire to make that the license for new code in
> > the tree.
> >
> > I would say this is a pretty good result.
> >
> > My ultimate suggestion is that we push the non-default copyright into
> > the impacted files, eg add short licenses headers to the man pages,
> > etc, and then delete the extraneous COPYING files once every single
> > file has a correct license statement. From that point we can look at
> > switching individual files based on the above Legal process to the
> > Default License, or just leave them as is - a historical quirk.
>
> It looks like a lot of work to do and it can be handled by anyone.
> Can OPA handle this?

Sorry,
OFA and not OPA.

>
> >
> > Jason



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v2] IB/{core,hw}: Add constant for node_desc
From: Yuval Shaia @ 2016-10-06  4:46 UTC (permalink / raw)
  To: Doug Ledford
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
	hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <c39be0ef-d170-d40d-aac4-0c157fc843b7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Aug 26, 2016 at 09:57:06AM -0400, Doug Ledford wrote:
> On 8/25/2016 1:57 PM, Yuval Shaia wrote:
> > Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > v0 -> v1:
> > 	* Exclude some files from commit based on Jason's comment
> > v1 -> v2:
> > 	* Re-include all the excluded files with BUILD_BUG_ON
> 
> v2 applied, thanks.

I have a bad feeling that this one slipped out.

> 
> 
> -- 
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG Key ID: 0E572FDD
> 



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

^ permalink raw reply

* Re: [PATCH rdma-core] Add a .travis.yml file
From: Leon Romanovsky @ 2016-10-06  4:53 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161005163250.GB18636-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

On Wed, Oct 05, 2016 at 10:32:50AM -0600, Jason Gunthorpe wrote:
> On Wed, Oct 05, 2016 at 02:51:32PM +0300, Leon Romanovsky wrote:
>
> > I'm using 16.04 and it has gcc 5.4.0 as a default. Don't you think that
> > 6.2 is too "new" for us and other distributions?
>
> I don't understand how a compiler can be too new? The next round of
> rolling release distros are likely to use this compiler, and those are
> the distros that will first pick to up rdma-core.

I have an issue in front of my eyes where kernel fails to build with old
gcc version, while new gcc version works.

I don't want the same situation here.

>
> > gcc (Ubuntu 5.4.0-6ubuntu1~16.04.2) 5.4.0 20160609
>
> This is exactly why the automatic builds should run the latest
> compiler, because people are not going to install it on their desktop.
>
> People will test with older compilers on their own. If we want to run
> automatic tests from travis with old compilers we can, but my results
> basically show that older compilers are just buggy and produce
> incorrect warnings.

My experience shows the same, however I want to minimize the test burden
on the users.

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH rdma-core] Add a .travis.yml file
From: Leon Romanovsky @ 2016-10-06  4:58 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <0de7bc15-5e2e-8240-ba84-a5a95dcb2834-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

On Wed, Oct 05, 2016 at 09:39:42AM -0700, Bart Van Assche wrote:
> On 10/05/2016 04:51 AM, Leon Romanovsky wrote:
> >On Wed, Sep 28, 2016 at 05:28:57PM -0600, Jason Gunthorpe wrote:
> >>Necessary to use the Travis CI service.
> >>
> >>Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> >>---
> >> .travis.yml | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >> create mode 100644 .travis.yml
> >>
> >>Now that travis is enabled on repo, we need a yml file to make it run.
> >>
> >>Here is a basic one.
> >>
> >>I don't test with distros as old as precise and I don't expect that glibc to
> >>be new enough to build this (eg missing timerfd, O_CLOEXEC, etc), and the
> >>cmake certainly isn't. So we use the trusty 'beta' environment and install
> >>gcc 6.2.
> >
> >I'm using 16.04 and it has gcc 5.4.0 as a default. Don't you think that
> >6.2 is too "new" for us and other distributions?
>
> Hi Leon,
>
> The rolling release distro that is running on my laptop includes gcc 6.2.1.

I'm not sure if there are many users of this bundle who are running
rolling distros.

Anyway, let's start from the simple (one, latest gcc) and increase
complexity later on.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox