Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1381681 > unrolled thread

Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Christoph Hellwig <hch@infradead.org> - 2016-04-18 15:10 +0200
    Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-18 19:50 +0200
      Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Christoph Hellwig <hch@infradead.org> - 2016-04-18 20:30 +0200
        Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Ira Weiny <ira.weiny@intel.com> - 2016-04-19 05:50 +0200
          Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Christoph Hellwig <hch@infradead.org> - 2016-04-19 20:50 +0200
        Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-19 19:40 +0200

#1381681 — Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access

On Thu, Apr 14, 2016 at 10:45:50AM -0600, Jason Gunthorpe wrote:
> On Thu, Apr 14, 2016 at 08:41:35AM -0700, Dennis Dalessandro wrote:
> > This patch series removes the write() interface for user access in favor of an
> > ioctl() based approach. This is in response to the complaint that we had
> > different handlers for write() and writev() doing different things and expecting
> > different types of data. See:
> 
> I think we should wait on applying these patches until we globally sort out
> what to do with the rdma uapi.
> 
> It just doesn't make alot of sense for drivers to have their own personal
> char devices. :(

I looked through the patches I tend to disagree - while we should wait
for a global UAPI for anything that's actually RDMA/verbs related these
seem to be misc little bits specific to the driver that have no business
in any sort of generic RDMA API.

> A second char dev for the eeprom? How is that OK? Why aren't you using
> the I2C layer for this?

... but this is a really good question, although the right layer to
plug this in would be the eeprom code in drivers/nvmem/

[toc] | [next] | [standalone]

#1381950

On Mon, Apr 18, 2016 at 06:09:09AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 14, 2016 at 10:45:50AM -0600, Jason Gunthorpe wrote:
> > On Thu, Apr 14, 2016 at 08:41:35AM -0700, Dennis Dalessandro wrote:
> > > This patch series removes the write() interface for user access in favor of an
> > > ioctl() based approach. This is in response to the complaint that we had
> > > different handlers for write() and writev() doing different things and expecting
> > > different types of data. See:
> > 
> > I think we should wait on applying these patches until we globally sort out
> > what to do with the rdma uapi.
> > 
> > It just doesn't make alot of sense for drivers to have their own personal
> > char devices. :(
> 
> I looked through the patches I tend to disagree - while we should wait
> for a global UAPI for anything that's actually RDMA/verbs related these
> seem to be misc little bits specific to the driver that have no business
> in any sort of generic RDMA API.

I wasn't arguing this should integrate into verbs in some way, only
that the way to access the driver-specific uAPI of a RDMA device should
be through the RDMA common uAPI and not through a random char dev.

.. and of course that the driver-specific API be subject to a sane
review and use of the normal standards, not just written off as
driver-garbage nobody cares about. :(

For instance, if we had a driver specific channel, it casts this
endless stream of uAPI verbs patches in a different light: maybe they
should go down the driver-specific channel instead.

Jason

[toc] | [prev] | [next] | [standalone]

#1381966

On Mon, Apr 18, 2016 at 11:40:47AM -0600, Jason Gunthorpe wrote:
> I wasn't arguing this should integrate into verbs in some way, only
> that the way to access the driver-specific uAPI of a RDMA device should
> be through the RDMA common uAPI and not through a random char dev.

Well, it's stuff not related to our RDMA userspace API (which _is_
Verbs, not counting for the complete crackpot abuse in usnic), but
very device specific. 

The stuff the intel driver are doing isn't pretty, but unfortunately
not unusual either - lots of SCSI or network driver have ioctls
like that.  Now we could argue if the ioctls should be one the
main node (uverbs) or the a driver private chardev, or not exist
at all and people will have to patch the driver with some vendor
version if they really need it.  Examples for either of these
choices exist in the tree.

[toc] | [prev] | [next] | [standalone]

#1382148

On Mon, Apr 18, 2016 at 11:24:11AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 18, 2016 at 11:40:47AM -0600, Jason Gunthorpe wrote:
> > I wasn't arguing this should integrate into verbs in some way, only
> > that the way to access the driver-specific uAPI of a RDMA device should
> > be through the RDMA common uAPI and not through a random char dev.
> 
> Well, it's stuff not related to our RDMA userspace API (which _is_
> Verbs, not counting for the complete crackpot abuse in usnic), but
> very device specific. 
> 
> The stuff the intel driver are doing isn't pretty, but unfortunately
> not unusual either - lots of SCSI or network driver have ioctls
> like that.  Now we could argue if the ioctls should be one the
> main node (uverbs) or the a driver private chardev, or not exist
> at all and people will have to patch the driver with some vendor
> version if they really need it.  Examples for either of these
> choices exist in the tree.

I'm a bit confused by what you are suggesting that "people will have to patch
the driver with some vendor version if they really need it."?

Could you elaborate?

PSM is the primary performant path for this device.  Without it this device is
severely limited in its intended functionality.

We are strongly motivated to have all of our functionality included in the
mainstream kernel.  So for eprom/snoop we would really like to find a way to
include all this functionality.

Ira

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

[toc] | [prev] | [next] | [standalone]

#1382789

On Mon, Apr 18, 2016 at 11:45:49PM -0400, Ira Weiny wrote:
> I'm a bit confused by what you are suggesting that "people will have to patch
> the driver with some vendor version if they really need it."?
> 
> Could you elaborate?

There are lots of drivers where we simply did not accept these vendor
specific extensions at all.  Especially for networking drivers it's
pretty common.  I'm not proposing this here, just saying that we have
lots of examples for it.

[toc] | [prev] | [next] | [standalone]

#1382754

On Mon, Apr 18, 2016 at 11:24:11AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 18, 2016 at 11:40:47AM -0600, Jason Gunthorpe wrote:
> > I wasn't arguing this should integrate into verbs in some way, only
> > that the way to access the driver-specific uAPI of a RDMA device should
> > be through the RDMA common uAPI and not through a random char dev.
> 
> Well, it's stuff not related to our RDMA userspace API (which _is_
> Verbs, not counting for the complete crackpot abuse in usnic), but
> very device specific. 

It is weakly related, it uses the same device discovery and security
model.

> The stuff the intel driver are doing isn't pretty, but unfortunately
> not unusual either - lots of SCSI or network driver have ioctls
> like that.  Now we could argue if the ioctls should be one the
> main node (uverbs) or the a driver private chardev, or not exist
> at all and people will have to patch the driver with some vendor
> version if they really need it.  Examples for either of these
> choices exist in the tree.

Right - and the RDMA uAPI has always had an integrated driver-bypass
channel as part of the verb uAPI calls, extending that to allow for
new-driver-specific calls seems very natural.

Jason

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web