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

Groups > linux.kernel > #1379051 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access Dennis Dalessandro <dennis.dalessandro@intel.com> - 2016-04-14 17:50 +0200
    Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-14 18:50 +0200
      Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Ira Weiny <ira.weiny@intel.com> - 2016-04-14 19:50 +0200
        Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-14 20:10 +0200
          Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access Dennis Dalessandro <dennis.dalessandro@intel.com> - 2016-04-14 20:50 +0200
            Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-14 21:00 +0200
        Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Leon Romanovsky <leon@leon.nu> - 2016-04-15 06:10 +0200
          Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Ira Weiny <ira.weiny@intel.com> - 2016-04-15 18:20 +0200
            Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Christoph Hellwig <hch@infradead.org> - 2016-04-15 19:40 +0200
              RE: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access "Hefty, Sean" <sean.hefty@intel.com> - 2016-04-15 19:50 +0200
              RE: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access "Woodruff, Robert J" <robert.j.woodruff@intel.com> - 2016-04-15 19:50 +0200
                Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Leon Romanovsky <leon@leon.nu> - 2016-04-15 23:30 +0200
              Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Leon Romanovsky <leon@leon.nu> - 2016-04-15 23:30 +0200
                Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Ira Weiny <ira.weiny@intel.com> - 2016-04-16 01:30 +0200
                  Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Leon Romanovsky <leon@leon.nu> - 2016-04-16 08:20 +0200
                    Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access Dennis Dalessandro <dennis.dalessandro@intel.com> - 2016-04-16 17:30 +0200
                Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-16 01:40 +0200
                  Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Leon Romanovsky <leon@leon.nu> - 2016-04-16 08:10 +0200
                    Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-16 21:20 +0200
                      Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access Dennis Dalessandro <dennis.dalessandro@intel.com> - 2016-04-18 14:10 +0200
            Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Leon Romanovsky <leon@leon.nu> - 2016-04-15 19:40 +0200
      Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access Dennis Dalessandro <dennis.dalessandro@intel.com> - 2016-04-14 20:00 +0200
        Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-14 20:50 +0200
        Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-20 22:40 +0200
          Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user access Dennis Dalessandro <dennis.dalessandro@intel.com> - 2016-04-22 20:40 +0200
            Re: [PATCH 0/7] IB/hfi1: Remove write() and use ioctl() for user  access Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-04-26 17:30 +0200
      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

Page 1 of 2  [1] 2  Next page →

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

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:

http://www.spinics.net/lists/linux-rdma/msg34451.html

In a response to that thread we mentioned some other possible approaches such
as using multiple files, or converting everything to writev(). However after
looking at things more it seemed a cleaner approach to use ioctl(). 

So each command that was being done through write() has been converted to
ioctl() and the write() interface is removed. We still use writev() for the data
path but control is done totally through ioctl().

The plan is to make the same sort of change in qib as well but we want to get
the opinion of the community on the approach first.

As part of this work I also decided to move the eprom functionality to its own
device (last patch) since it really is its own thing. It uses ioctl() as well,
again because it seems to be a more natural interface for this operation.

There is also a driver software version being exported via a sysfs file. This is
needed so that user space applications (psm) can  determine if it needs to do
ioctl() or write().

This patch applies on the latest hfi1 patch set "Fix link and other issues".

Patches can also be viewed on GitHub at:
https://github.com/ddalessa/kernel/tree/for-4.7

---

Dennis Dalessandro (7):
      IB/hfi1: Export drivers user sw version via sysfs
      IB/hfi1: Remove unused user command
      IB/hfi1: Add ioctl() interface for user commands
      IB/hfi1: Remove write(), use ioctl() for user cmds
      IB/hfi1: Add trace message in user IOCTL handling
      IB/hfi1: Consolidate IOCTL defines
      IB/hfi1: Move eprom to its own device


 drivers/staging/rdma/hfi1/common.h   |    3 
 drivers/staging/rdma/hfi1/diag.c     |  170 ++++++++++++++++++---------
 drivers/staging/rdma/hfi1/eprom.c    |  121 +------------------
 drivers/staging/rdma/hfi1/eprom.h    |   16 ++-
 drivers/staging/rdma/hfi1/file_ops.c |  213 ++++++++++++++--------------------
 drivers/staging/rdma/hfi1/hfi.h      |   22 +++-
 drivers/staging/rdma/hfi1/sysfs.c    |    8 +
 drivers/staging/rdma/hfi1/trace.c    |    1 
 drivers/staging/rdma/hfi1/trace.h    |    1 
 include/uapi/rdma/hfi/hfi1_user.h    |  106 ++++++++++++++++-
 10 files changed, 350 insertions(+), 311 deletions(-)

-- 
-Denny

[toc] | [next] | [standalone]

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

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. :(

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

Why is there a snoop interface in here? How is that not something that
belongs in a the core code?

Jason

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

#1379158 — 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'm afraid I have to disagree at this time.  Someday we may have "1 char device
to rule them all" but right now we don't have any line of sight to that
solution.  It may be _years_ before we can agree to the semantics which will
work for all high speed, kernel bypass, rdma, low latency, network devices.

We need to fix the write/writev problem now.[1]

Ira

[1] https://www.spinics.net/lists/linux-rdma/msg34451.html

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

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

On Thu, Apr 14, 2016 at 01:48:31PM -0400, Ira Weiny 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'm afraid I have to disagree at this time.  Someday we may have "1 char device
> to rule them all" but right now we don't have any line of sight to that
> solution.  It may be _years_ before we can agree to the semantics which will
> work for all high speed, kernel bypass, rdma, low latency, network devices.

There are some pretty obvious paths to make this saner that could only
be a few weeks away, we haven't even had the first conversations
yet. I think you are completely wrong there is no 'line of sight'

It certainly can't be years.

There is some rational for a very driver specific thing, but EEPROM
and snoop? Seriously?

Jason

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

#1379216

On Thu, Apr 14, 2016 at 12:05:40PM -0600, Jason Gunthorpe wrote:
>There are some pretty obvious paths to make this saner that could only
>be a few weeks away, we haven't even had the first conversations
>yet. I think you are completely wrong there is no 'line of sight'
>
>It certainly can't be years.

Does fixing the current write()/writev() problem have any real impact on how 
we proceed for the "1 char dev to rule them all" idea?

>There is some rational for a very driver specific thing, but EEPROM
>and snoop? Seriously?

That's the thing, I think these are very driver specific [1]. I'm not dead 
set that the eprom needs to be its own device, it made sense to me, but if 
others feel the handling should be back in the hfi1 char device I'm fine 
with that.

As for the snoop stuff, perhaps that would be better in rdmavt?

[1] http://marc.info/?l=linux-rdma&m=146065638629146&w=2

-Denny

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

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

On Thu, Apr 14, 2016 at 02:42:01PM -0400, Dennis Dalessandro wrote:
> >It certainly can't be years.
> 
> Does fixing the current write()/writev() problem have any real
> impact on how we proceed for the "1 char dev to rule them all" idea?

We aren't going to take a bad uAPI into mainline. So how many times do
you want to redo the userspace? I have no objection to the patch
landing, just as long as it stays in staging until we have the uAPI
discussion as a community.

As for the 'one char device', I actually think it would be really
simple.

Add a new uverbs ioctl:

 int hfi1_fd = ioctl(uverbs_fd, RDMA_GET_DRIVER_OPS_FD, "psm2.intel.com");

 ioctl(hfi1_fd, HFI1_IOCTL_ASSIGN_CTXT, ...);
 write(hfi1_fd, ...);

At least that gives us far better options for discovery and versioning
of this stuff than a driver-specific char device.

[eg this would use anon_inode_getfile, like event fds, completion
channels, etc]

You guys need this the most, propose something already.

* driver specific ioctls might be nicer, but people argue that is not
  performant enough for what you want... Unclear to me.

Jason

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

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

[Multipart message — attachments visible in raw view] — view raw

On Thu, Apr 14, 2016 at 01:48:31PM -0400, Ira Weiny 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'm afraid I have to disagree at this time.  Someday we may have "1 char device
> to rule them all" but right now we don't have any line of sight to that
> solution.  It may be _years_ before we can agree to the semantics which will
> work for all high speed, kernel bypass, rdma, low latency, network devices.

You didn't ever try to come and work on the solution. We talked about
finite time frame (_months_) which is doable based on knowledge that user
space parts are developed by the same companies and all our future changes
will be in one subsystem.

You were supposed to prepare "wish list" from this new API as an initial
phase. If you do it, you will find that it is very short and in the
initial meeting you will see that it similar to other participants in
linux-rdma community.

> 
> We need to fix the write/writev problem now.[1]

No, this driver in staging and the proper way to move it out will be to
converge on common API and one clear path instead of duplicating the
interfaces and "inventing the wheel".

> 
> Ira
> 
> [1] https://www.spinics.net/lists/linux-rdma/msg34451.html
> 
> --
> 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]

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

On Fri, Apr 15, 2016 at 07:01:26AM +0300, Leon Romanovsky wrote:
> On Thu, Apr 14, 2016 at 01:48:31PM -0400, Ira Weiny 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'm afraid I have to disagree at this time.  Someday we may have "1 char device
> > to rule them all" but right now we don't have any line of sight to that
> > solution.  It may be _years_ before we can agree to the semantics which will
> > work for all high speed, kernel bypass, rdma, low latency, network devices.
> 
> You didn't ever try to come and work on the solution. We talked about
> finite time frame (_months_) which is doable based on knowledge that user
> space parts are developed by the same companies and all our future changes
> will be in one subsystem.

How can you say that I am not working on a solution?

We spent most of last week discussing possible solutions and I am in support of
a more common core.  But ask yourself this.


If hfi1 did not support verbs at all would this even be an issue?


> 
> You were supposed to prepare "wish list" from this new API as an initial
> phase. If you do it, you will find that it is very short and in the
> initial meeting you will see that it similar to other participants in
> linux-rdma community.

The list of operations may be short.  But the way in which you do those in a
performant way for each hardware device is _very_ different.  This is a problem
which has been debated for years and no one has come up with an elegant
solution.  Every solution ends up being, to quote a presenter at last weeks
conference, "shoving a square peg into a round hole".

Until we all admit 2 things.

	1) That there are devices which don't operate on QPs
	2) That the High Speed interconnect core should present something more
	   abstract than a QP interface

we are not really creating a common layer.

I do admit Jasons idea has some merit but I'm just not sure it provides so much
benefit that it is worth the effort at this time.

Ira

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

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

On Fri, Apr 15, 2016 at 08:30:35PM +0300, Leon Romanovsky wrote:
> Great, did you show it to other RDMA stakeholders except Intel?
> I saw nothing posted on ML or proposed for initial discussion, which
> will be held in the next week or two.

I fear it's kfabrics, which is an entirely crackpot idea and a total
non-starter, but for some reason Intel and their buddies keep wasting
time on it.

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

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

> > Great, did you show it to other RDMA stakeholders except Intel?
> > I saw nothing posted on ML or proposed for initial discussion, which
> > will be held in the next week or two.
> 
> I fear it's kfabrics, which is an entirely crackpot idea and a total
> non-starter, but for some reason Intel and their buddies keep wasting
> time on it.

There were discussions between several developers from multiple companies around moving away from using writev.  That's it.  Making random accusations and throwing crap over the wall does nothing to improve the community or instill trust.

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

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

> I fear it's kfabrics, which is an entirely crackpot idea and a total non-starter, but for some reason Intel and their buddies keep wasting time on it.

What is being discussed her is not kfabrics. That is a totally different out of kernel pathfinding project at this point.
What is being discussed here is how to best solve the write/writev issue with the PSM interface. The code submitted was to move
to IOCTL instead, but people like Jason have suggested routing the IOCTLs through the verbs layer instead.

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

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

[Multipart message — attachments visible in raw view] — view raw

On Fri, Apr 15, 2016 at 05:44:48PM +0000, Woodruff, Robert J wrote:
> > I fear it's kfabrics, which is an entirely crackpot idea and a total non-starter, but for some reason Intel and their buddies keep wasting time on it.
> 
> What is being discussed her is not kfabrics. That is a totally different out of kernel pathfinding project at this point.
> What is being discussed here is how to best solve the write/writev issue with the PSM interface. The code submitted was to move
> to IOCTL instead, but people like Jason have suggested routing the IOCTLs through the verbs layer instead.

The discussion here is much broader than conversion of PSM interface.

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

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

[Multipart message — attachments visible in raw view] — view raw

On Fri, Apr 15, 2016 at 10:34:01AM -0700, Christoph Hellwig wrote:
> On Fri, Apr 15, 2016 at 08:30:35PM +0300, Leon Romanovsky wrote:
> > Great, did you show it to other RDMA stakeholders except Intel?
> > I saw nothing posted on ML or proposed for initial discussion, which
> > will be held in the next week or two.
> 
> I fear it's kfabrics, which is an entirely crackpot idea and a total
> non-starter, but for some reason Intel and their buddies keep wasting
> time on it.

It is a different thing, during OFA16 conference we were **strongly
advised** to move from old read/write interface in RDMA stack to
something else.

The agreement was that a couple of weeks after the conference,
Liran will organize open web meeting to discuss what we want from
this interface.

It is important to make it open, so all participants will be able to
express their willingness.

Intel as usual decided to do it in their way and the result is presented
on this mailing list.

Thanks.

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

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

On Sat, Apr 16, 2016 at 12:23:28AM +0300, Leon Romanovsky wrote:

> 
> Intel as usual decided to do it in their way and the result is presented
> on this mailing list.

Excuse me, but this statement is completely unfair.  We were specifically asked
by Al and Linus to fix our char device with regards to the write/writev
inconsistency.

https://www.spinics.net/lists/linux-rdma/msg34451.html

Which is _exactly_ what this patch series does.

Do you have a technical reason that this patch series does not fix the
write/writev issue brought up by Al?

Ira

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

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

[Multipart message — attachments visible in raw view] — view raw

On Fri, Apr 15, 2016 at 07:28:01PM -0400, Ira Weiny wrote:
> On Sat, Apr 16, 2016 at 12:23:28AM +0300, Leon Romanovsky wrote:
> Do you have a technical reason that this patch series does not fix the
> write/writev issue brought up by Al?

Sure, I truly believe that we can do common API in a months time-frame
and I want to be focused on one transition path only (write/read -> new
API) and not on two parallel paths (ioctl -> new API and write/read ->
new API) plus support of all these intermediate steps.

The original request came after this driver was moved from staging to
RDMA stack, since the driver is still in staging, there is no need to
hurry up now.

> 
> Ira
> 

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

#1380561

On Sat, Apr 16, 2016 at 09:09:40AM +0300, Leon Romanovsky wrote:
>On Fri, Apr 15, 2016 at 07:28:01PM -0400, Ira Weiny wrote:
>> On Sat, Apr 16, 2016 at 12:23:28AM +0300, Leon Romanovsky wrote:
>> Do you have a technical reason that this patch series does not fix the
>> write/writev issue brought up by Al?
>
>Sure, I truly believe that we can do common API in a months time-frame
>and I want to be focused on one transition path only (write/read -> new
>API) and not on two parallel paths (ioctl -> new API and write/read ->
>new API) plus support of all these intermediate steps.

That doesn't say anything about how this patch doesn't address Al and 
Linus's complaint, or raise a technical issue with the patch set.

These are two separate issues. I do not see a reason to try and make them 
one, and use this to drive the "one-device to rule them all" idea.  This 
series converts the write() to ioctl() and fixes the problem we set to, as 
promised. You don't like the API, that's fine.  We'll discuss that on 
linux-rdma, but no reason to hold this patch set while that happens. 

>The original request came after this driver was moved from staging to
>RDMA stack, since the driver is still in staging, there is no need to
>hurry up now.

There is no need to keep the driver in staging. This is not a driver that 
has style problems or is not well tested.  It is a driver that has been 
heavily tested, performs well and has completed its staging TODO list. We 
went ahead and added this write()/writev() fix before making the move 
because Al and Linus wanted that issue addressed. For the record:

 $ cat drivers/staging/rdma/hfi1/TODO
 July, 2015

 - Remove unneeded file entries in sysfs
 - Remove software processing of IB protocol and place in library for use
   by qib, ipath (if still present), hfi1, and eventually soft-roce

Both of those items are complete. The API issue was raised back when the 
driver was submitted (almost a year ago), as you can see it did not make the 
cut as a staging requirement. Whether you agree with the maintainer's 
decision or not. I don't see how it's fair to try and add it again now.

As I mentioned let's discuss the uAPI stuff on linux-rdma. Have the web 
meetings that you were mentioning and do whatever we need to in order to 
improve the sub-system, but stop trying to tie our driver and moving out of 
staging to this much larger issue.

Thanks

-Denny

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

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

On Sat, Apr 16, 2016 at 12:23:28AM +0300, Leon Romanovsky wrote:

> Intel as usual decided to do it in their way and the result is presented
> on this mailing list.

Dennis was pretty clear he was going to send the patches to address
Al's concern, which he has done.

I was also pretty clear I was looking to get rid of the char dev :)

Not seeing a problem here.

Jason

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

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

[Multipart message — attachments visible in raw view] — view raw

On Fri, Apr 15, 2016 at 05:37:32PM -0600, Jason Gunthorpe wrote:
> On Sat, Apr 16, 2016 at 12:23:28AM +0300, Leon Romanovsky wrote:
> 
> > Intel as usual decided to do it in their way and the result is presented
> > on this mailing list.
> 
> Dennis was pretty clear he was going to send the patches to address
> Al's concern, which he has done.
> 
> I was also pretty clear I was looking to get rid of the char dev :)

Yes, and I was pretty clear that we need to converge on one common API
prior to converting old code (including drivers in staging) in order to
do it once only.

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

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

On Sat, Apr 16, 2016 at 09:00:42AM +0300, Leon Romanovsky wrote:
> On Fri, Apr 15, 2016 at 05:37:32PM -0600, Jason Gunthorpe wrote:
> > On Sat, Apr 16, 2016 at 12:23:28AM +0300, Leon Romanovsky wrote:
> > 
> > > Intel as usual decided to do it in their way and the result is presented
> > > on this mailing list.
> > 
> > Dennis was pretty clear he was going to send the patches to address
> > Al's concern, which he has done.
> > 
> > I was also pretty clear I was looking to get rid of the char dev :)
> 
> Yes, and I was pretty clear that we need to converge on one common API
> prior to converting old code (including drivers in staging) in order to
> do it once only.

While we are at it, could the person who'd come up with ui_lseek() be located
and made to stand up and explain the rationale behind the SEEK_END semantics
therein?  To quote the manpage (and paraphrase just about any introductory
textbook):
       SEEK_END
              The file offset is set to the size of the file plus offset bytes.

I'm really curious - which part of "plus" might have lead to
        case SEEK_END:
                offset = ((dd->kregend - dd->kregbase) + DC8051_DATA_MEM_SIZE) -
                        offset;
and, if its author has decided that of course it _must_ have meant "minus",
why had he or she failed to post a correction to the manpage?  Or, on the
off-chance that this "plus" might have something to do with reality,
experimented with some file, for that matter.

Folks, this is a well-earned "F".  And not just for Unix Programming 101 -
the same semantics applies to fseek(3), which is a part of C standard.
Incidentally, lseek(fd, 0, SEEK_END) is "seek to end", not "fail with EINVAL".

As for the use of ioctl...  Frankly, considering the above, it does sound like
"that'll make them STFU about the weirdness - ioctl *is* weird, so there!"

Single-consumer APIs stink, film at 11...

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

#1381645

On Sat, Apr 16, 2016 at 08:19:17PM +0100, Al Viro wrote:

>While we are at it, could the person who'd come up with ui_lseek() be located
>and made to stand up and explain the rationale behind the SEEK_END semantics
>therein?  To quote the manpage (and paraphrase just about any introductory
>textbook):
>       SEEK_END
>              The file offset is set to the size of the file plus offset bytes.
>
>I'm really curious - which part of "plus" might have lead to
>        case SEEK_END:
>                offset = ((dd->kregend - dd->kregbase) + DC8051_DATA_MEM_SIZE) -
>                        offset;
>and, if its author has decided that of course it _must_ have meant "minus",
>why had he or she failed to post a correction to the manpage?  Or, on the

Original author of that code confirmed it is just a coding mistake and we 
will fix it.

-Denny

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web