All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@mellanox.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: Doug Ledford <dledford@redhat.com>,
	RDMA mailing list <linux-rdma@vger.kernel.org>,
	Haggai Eran <haggaie@mellanox.com>,
	Saeed Mahameed <saeedm@mellanox.com>,
	linux-netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH rdma-next 2/5] RDMA/core: Don't depend device ODP capabilities on kconfig option
Date: Thu, 20 Dec 2018 23:22:53 +0000	[thread overview]
Message-ID: <20181220232247.GV29267@mellanox.com> (raw)
In-Reply-To: <20181220175020.GD3940@mtr-leonro.mtl.com>

On Thu, Dec 20, 2018 at 07:50:20PM +0200, Leon Romanovsky wrote:
> > > > > -#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> > > > >  	if (MLX5_CAP_GEN(mdev, pg))
> > > > >  		props->device_cap_flags |= IB_DEVICE_ON_DEMAND_PAGING;
> > > > >  	props->odp_caps = dev->odp_caps;
> > > > > -#endif
> > > >
> > > > But shouldn't this be protected? If the driver has compiled out ODP it
> > > > shouldn't set the cap flag...
> > >
> > > I see those capabilities as device properties and not as kernel ones.
> > >
> > > Current situation looks bad for me, when I have same device which
> > > reports differently information depends on some compilation flag.
> >
> > It is not device capabilities, it is a kernel API capability if the
> > API is not available the bit should not be set.
> 
> Is it better? Should I resend the series?

Hrm.. I think I'm inclined to keep the the if() in the same place as
the ifdefs. We can revise it later.

I noticed other busted up stuff:
 - IB_DEVICE_ON_DEMAND_PAGING is apparently UAPI but not in a uapi
   header (grr)
 - ucontext shouldn't have a driver callback, that should be moved to
   ops (ie to mlx5_ib_dev_odp_ops)
 - IB_DEVICE_ON_DEMAND_PAGING should be set in core code based
   on ops.invalidate_range being present, drivers shouldn't set it..

Jason

  reply	other threads:[~2018-12-20 23:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-20  9:23 [PATCH rdma-next 0/5] Cleanup of CONFIG_INFINIBAND_ON_DEMAND_PAGING usage Leon Romanovsky
2018-12-20  9:23 ` [PATCH rdma-next 1/5] RDMA: Clean structures from CONFIG_INFINIBAND_ON_DEMAND_PAGING Leon Romanovsky
2018-12-20  9:23 ` [PATCH rdma-next 2/5] RDMA/core: Don't depend device ODP capabilities on kconfig option Leon Romanovsky
2018-12-20 17:20   ` Jason Gunthorpe
2018-12-20 17:29     ` Leon Romanovsky
2018-12-20 17:33       ` Jason Gunthorpe
2018-12-20 17:50         ` Leon Romanovsky
2018-12-20 23:22           ` Jason Gunthorpe [this message]
2018-12-20  9:23 ` [PATCH rdma-next 3/5] RDMA/mlx5: Introduce and reuse helper to identify ODP MR Leon Romanovsky
2018-12-20  9:23 ` [PATCH rdma-next 4/5] RDMA/mlx5: Embed into the code flow the ODP config option Leon Romanovsky
2018-12-20  9:23 ` [PATCH mlx5-next 5/5] RDMA/mlx5: Delete declaration of already removed function Leon Romanovsky
2018-12-21  3:32 ` [PATCH rdma-next 0/5] Cleanup of CONFIG_INFINIBAND_ON_DEMAND_PAGING usage Jason Gunthorpe
2018-12-21 13:59   ` Leon Romanovsky
2018-12-21 16:59     ` Jason Gunthorpe
2018-12-22  9:18       ` Leon Romanovsky

Reply instructions:

You may reply publicly 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=20181220232247.GV29267@mellanox.com \
    --to=jgg@mellanox.com \
    --cc=dledford@redhat.com \
    --cc=haggaie@mellanox.com \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@mellanox.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.