Skip to content

L
linux
Switch branch/tag
  1. 31 Jul, 2018 12 commits
  3. 27 Jul, 2018 3 commits
  5. 26 Jul, 2018 14 commits
  7. 25 Jul, 2018 11 commits
    • Varsha Rao's avatar
      IB/core: Remove extra parentheses · 076dd53b
      Varsha Rao authored 
      Remove unnecessary parentheses to fix the clang warning of extraneous
parentheses.
Signed-off-by: default avatarVarsha Rao <rvarsha016@gmail.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      076dd53b
    • Bart Van Assche's avatar
      RDMA/ocrdma: Suppress a compiler warning · 9491a1ed
      Bart Van Assche authored 
      This patch avoids that the following compiler warning is reported when
building with gcc 8 and W=1:

In function 'ocrdma_mbx_get_ctrl_attribs',
    inlined from 'ocrdma_init_hw' at drivers/infiniband/hw/ocrdma/ocrdma_hw.c:3224:11:
drivers/infiniband/hw/ocrdma/ocrdma_hw.c:1368:3: warning: 'strncpy' output may be truncated copying 31 bytes from a string of length 31 [-Wstringop-truncation]
   strncpy(dev->model_number,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~
    hba_attribs->controller_model_number, 31);
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: default avatarBart Van Assche <bart.vanassche@wdc.com>
Acked-by: default avatarSelvin Xavier <selvin.xavier@broadcom.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      9491a1ed
    • Jason Gunthorpe's avatar
      IB/uverbs: Fix locking around struct ib_uverbs_file ucontext · 22fa27fb
      Jason Gunthorpe authored 
      We have a parallel unlocked reader and writer with ib_uverbs_get_context()
vs everything else, and nothing guarantees this works properly.

Audit and fix all of the places that access ucontext to use one of the
following locking schemes:
- Call ib_uverbs_get_ucontext() under SRCU and check for failure
- Access the ucontext through an struct ib_uobject context member
  while holding a READ or WRITE lock on the uobject.
  This value cannot be NULL and has no race.
- Hold the ucontext_lock and check for ufile->ucontext !NULL

This also re-implements ib_uverbs_get_ucontext() in a way that is safe
against concurrent ib_uverbs_get_context() and disassociation.

As a side effect, every access to ucontext in the commands is via
ib_uverbs_get_context() with an error check, or via the uobject, so there
is no longer any need for the core code to check ucontext on every command
call. These checks are also removed.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      22fa27fb
    • Jason Gunthorpe's avatar
      IB/mlx5: Use the ucontext from the uobj, not the file · c36ee46d
      Jason Gunthorpe authored 
      This approach matches the standard flow of the typical write method that
relies on the HW object to store the device and the uobject to access the
ucontext.  Avoids the use of the devx_ufile2uctx in several places will
make revising the semantics of ib_uverbs_get_ucontext() in the next patch
simpler.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      c36ee46d
    • Jason Gunthorpe's avatar
      IB/uverbs: Move the FD uobj type struct file allocation to alloc_commit · aba94548
      Jason Gunthorpe authored 
      Allocating the struct file during alloc_begin creates this strange
asymmetry with IDR, where the FD has two krefs pointing at it during the
pre-commit phase. In particular this makes the abort process for FD very
strange and confusing.

For instance abort currently calls the type's destroy_object twice, and
the fops release once if abort is done. This is very counter intuitive. No
fops should be called until alloc_commit succeeds, and destroy_object
should only ever be called once.

Moving the struct file allocation to the alloc_commit is now simple, as we
already support failure of rdma_alloc_commit_uobject, with all the
required rollback pieces.

This creates an understandable symmetry with IDR and simplifies/fixes the
abort handling for FD types.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      aba94548
    • Jason Gunthorpe's avatar
      IB/uverbs: Always propagate errors from rdma_alloc_commit_uobject() · 2c96eb7d
      Jason Gunthorpe authored 
      The ioctl framework already does this correctly, but the write path did
not. This is trivially fixed by simply using a standard pattern to return
uobj_alloc_commit() as the last statement in every function.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      2c96eb7d
    • Jason Gunthorpe's avatar
      IB/uverbs: Rework the locking for cleaning up the ucontext · e951747a
      Jason Gunthorpe authored 
      The locking here has always been a bit crazy and spread out, upon some
careful analysis we can simplify things.

Create a single function uverbs_destroy_ufile_hw() that internally handles
all locking. This pulls together pieces of this process that were
sprinkled all over the places into one place, and covers them with one
lock.

This eliminates several duplicate/confusing locks and makes the control
flow in ib_uverbs_close() and ib_uverbs_free_hw_resources() extremely
simple.

Unfortunately we have to keep an extra mutex, ucontext_lock.  This lock is
logically part of the rwsem and provides the 'down write, fail if write
locked, wait if read locked' semantic we require.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      e951747a
    • Jason Gunthorpe's avatar
      IB/uverbs: Revise and clarify the rwsem and uobjects_lock · 87064277
      Jason Gunthorpe authored 
      Rename 'cleanup_rwsem' to 'hw_destroy_rwsem' which is held across any call
to the type destroy function (aka 'hw' destroy). The main purpose of this
lock is to prevent normal add and destroy from running concurrently with
uverbs_cleanup_ufile()

Since the uobjects list is always manipulated under the 'hw_destroy_rwsem'
we can eliminate the uobjects_lock in the cleanup function. This allows
converting that lock to a very simple spinlock with a narrow critical
section.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      87064277
    • Jason Gunthorpe's avatar
      IB/uverbs: Clarify and revise uverbs_close_fd · e6d5d5dd
      Jason Gunthorpe authored 
      The locking requirements here have changed slightly now that we can rely
on the ib_uverbs_file always existing and containing all the necessary
locking infrastructure.

That means we can get rid of the cleanup_mutex usage (this was protecting
the check on !uboj->context).

Otherwise, follow the same pattern that IDR uses for destroy, acquire
exclusive write access, then call destroy and the undo the 'lookup'.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      e6d5d5dd
    • Jason Gunthorpe's avatar
      IB/uverbs: Revise the placement of get/puts on uobject · 5671f79b
      Jason Gunthorpe authored 
      This wasn't wrong, but the placement of two krefs didn't make any
sense. Follow some simple rules.

- A kref is held inside uobjects_list
- A kref is held inside the IDR
- A kref is held inside file->private
- A stack based kref is passed bettwen alloc_begin and
  alloc_abort/alloc_commit

Any place we destroy one of the above pointers, we stick a put,
or 'move' the kref into another pointer.

The key functions have sensible semantics:
- alloc_uobj fully initializes the common members in uobj, including
  the list
- Get rid of the uverbs_idr_remove_uobj helper since IDR remove
  does require put, but it depends on the situation. Later
  patches will re-consolidate this differently.
- alloc_abort always consumes the passed kref, done in the type
- alloc_commit always consumes the passed kref, done in the type
- rdma_remove_commit_uobject always pairs with a lookup_get

After it is all done the only control flow change is to:
- move a get from alloc_commit_fd_uobject to rdma_alloc_commit_uobject
- add a put to remove_commit_idr_uobject
- Consistenly use rdma_lookup_put in rdma_remove_commit_uobject at
  the right place
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      5671f79b
    • Jason Gunthorpe's avatar
      IB/uverbs: Clarify the kref'ing ordering for alloc_commit · c561c288
      Jason Gunthorpe authored 
      The alloc_commit callback makes the uobj visible to other threads,
and it does so using a 'move' semantic of the uobj kref on the stack
into the public storage (eg the IDR, uobject list and file_private_data)

Once this is done another thread could start up and trigger deletion
of the kref. Fortunately cleanup_rwsem happens to prevent this from
being a bug, but that is a fantastically unclear side effect.

Re-organize things so that alloc_commit is that last thing to touch
the uobj, get rid of the sneaky implicit dependency on cleanup_rwsem,
and add a comment reminding that uobj is no longer kref'd after
alloc_commit.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
      c561c288
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7